Virtual Curiosities

Fixing Copilot's Code by Learning Rust

Share
In this article, I'll document my journey fixing the code that Copilot generated for me yesterday when I coded with AI for the first time. I'll do this by actually learning Rust using the online book instead of asking Copilot to do everything for me.

New Branch

The first thing to do is create a new Git branch for this clean code since I want to keep the master branch the way it is. I already published this experiment to Github so I wouldn't to overwrite it by accident.

Disable Inlay Hints

Next I need to get rid of the rust-analyzer types that appear in front of every variable since they are very distracting. I keep mistaking them for text I have typed. Fortunately this can be done through the extension's settings in VS Code. Just disable all inlay hints (there are several, for types, closing braces, parameters, etc.).

A Class for The I/O: Rust's Module System?

Seeing the code we've generated to see what is the easiest to fix, I notice that I have a few lines to get the window, document, and body element from the web_sys API.

// Get the window and document objects
let window = window().expect("should have a window");
let document = window.document().expect("should have a document");
let body = document.body().expect("should have a body");

This should certainly be its own class so we can pass these variables around to whatever function needs to create divs and other frontend things.

File Naming Convention

A new class means a new file, and a new file means a filename. The first thing to learn is what should files be called in Rust. Does it have a naming convention? In some languages, like Python, it's all lower case, no dashes, no underscore, while others, like Typescript, are fine with PascalCase.

It seems Rust uses snake_case for "Modules" which I assume are the files you can import.

At first I called the class Document but seeing as web_sys::Document already exists I renamed it to Page, so the new file was called src/page.rs.

Export in Rust

In Rust, declared classes are private by default. To make it available to other files we must use the pub keyword. The same applies to fields of those classes.

pub struct Page {
    pub window: web_sys::Window,
    pub document: web_sys::Document,
    pub body: web_sys::HtmlElement,
}

By the way I still have Copilot enabled and it tired to create methods like get_window to make the once private window field available publicly. Naturally I deleted that code because there isn't much need for getters and setters here.

Importing Files in Rust

Intriguingly, it seems one of the weird things I noticed about Rust is actually normal and not an oddity produced by Copilot: to import a file, you need both mod AND use. However, what I didn't understand at the time was that lib.rs is the starting point for the Rust compile in a library with no entry point, and that's where the mod declarations should be. In order words, you're supposed to put the mod in lib.rs, but you shouldn't have any use in lib.rs so you can make it just the file that lists all your modules, without any function definitions or structures in it.

mod page;

use page::{Page};

So you need both of these to use the page module in lib.rs, but if you have mod page in lib.rs you can use use page on any of your modules without having to write mod page as well.

Constructors and Instantiating Classes

Rust doesn't have a constructor. By convention a static method called new is the constructor, but you can't new Page, you just Page::new if you have a new static method in impl Page.

Returning Last Line by Default?

In Rust, the last line of a function is returned if it doesn't end in a semicolon (;). That's very weird and I find myself adding the semicolon just to be told that the function isn't returning anything. But it's the Rust way, I guess.

Expect

While vibe coding with Copilot I found it odd that every single statement Copilot produced ended in expect. Does Rust really need this every single time? It turns out that if you use expect the program will just crash. This is okay, I guess, since there really isn't any way for a web assembly program that needs to display something on a webpage to recover from not being able to get the webpage's body element. However, for the sake of making reusable code cleaner, we're going to move this "panic" error to the caller.

Rust provides several ways to return errors and missing values, and a shorthand way to propagate them without having to write if every time. This way is the question mark (?). If a function returns an Option<T>, for example, that may mean you get a T or nothing on error. You can propagate this nothingness upwards like this:

impl Page {
    pub fn new(window: web_sys::Window) -> Option<Self> {
        let document = window.document()?;
        let body = document.body()?;
        Some(Page { window, document, body })
    }

    pub fn from_global() -> Option<Self> {
        let window = web_sys::window()?;
        Page::new(window)
    }
}

Above, Some(Page...) returns an Option<Page> that isn't empty. If, for example, web_sys::window() returns an empty Option, then Page::new won't be executed because the empty Option will be returned on that line due to the ?.

Generic Div Making Function

As a browser-based game, Pong needs to create several div elements from Rust and the code to create even a single one is verbose. It's verbose even in Javascript, which is why everybody uses React, or JQuery, instead of VanillaJS. But there is a trick that few web developers know that can help us with this verbosity: we can just create a function for that.

So instead of this mess every time:

let ball_div = document.create_element("div").expect("should create a div");
ball_div.set_class_name("ball");
field_div.append_child(&ball_div).expect("should append the div");

We can just put this in a method on our new Page class:

impl Page {
    pub fn element(&self, tag: &str, cls: &str) -> Result<Element, wasm_bindgen::JsValue> {
        // Create a new element with the specified tag and class
        let element = self.document.create_element(tag)?;
        element.set_class_name(cls);

        // Append the new element to the body
        self.body.append_child(&element)?;

        // Return the created element
        Ok(element)
    }

    pub fn element_with_parent(&self, tag: &str, cls: &str, parent: &Element) -> Result<Element, wasm_bindgen::JsValue> {
        let element = self.element(tag, cls)?;
        parent.append_child(&element)?;
        Ok(element)
    }
}

Now we can call this instead every time.

let field_div = page.element("div", "field")?;
page.body.append_child(&field_div)?;

let ball_div = page.element_with_parent("div", "ball", &field_div)?;
let left_pad_div = page.element_with_parent("div", "pad", &field_div)?;
let right_pad_div = page.element_with_parent("div", "pad", &field_div)?;
let left_score_div = page.element_with_parent("div", "score left", &field_div)?;
let right_score_div = page.element_with_parent("div", "score right", &field_div)?;
let speed_counter_div = page.element_with_parent("div", "speed_counter", &field_div)?;

For some reason I can't use page.body because it's of type HtmlElement instead of Element. I'm not sure how to fix that.

Borrowing Values

It seems the & operator I was confused about before is the operator for borrowing. When an argument of a function has a type prefixed by &, that means it's a "reference," which I assume means the value is passed by reference instead of copied by value.

That means that like in C++ I'll want to use this & everywhere.

It seems we can also have mutable references with &mut. This is probably an important part of the puzzle I was missing before as it wasn't included in any snippet that Copilot generated.

Game Class

I can't actually use ? in my main function since it doesn't return an option. This sounds like a good time to crate a class to hold my Pong game divs. I already have a Pong class, so I guess I'll just use it and move all this code that is in lib.rs to it.

I notice something odd. It seems ? only works if the return type of a function matches the return type of the function being called. For example, if a function return an empty Option<T> on error and another returns a Result<T, E> on error, you can't use ? with both of them because your function needs to return either an Option or a Result. I'm not sure what to do in these cases yet.

Setting Position and Size

Although it's called "set position and size" of a div, this is in practice the draw() method of the game. We might want to change how we render elements some day, so let's create a separate refresh function for each game object.

pub fn refresh_field(&self) {
    set_position_and_size(
        &self.field_div,
        self.field.top_left.x,
        self.field.top_left.y,
        self.field.size.x,
        self.field.size.y,
    );
}

This function that Copilot wrote for me is clearly not ideal since it doesn't use the geometry code I wrote. It was fine when I was letting Copilot write the code, but it's not okay when I have to type all these arguments myself.

First I'll add some constructors to the Rect and Point class. Copilot is really good at generating these.

Now I'll make this a method of Rect. Actually it's better if its own module. This is a void function but it can fail, so I guess I use Result<void>? Let's check the book... oh, it's Result<()>. That's rather unexpected.

Wait a minute, why am I setting the style as a string with format! again? How do I set the style by property the normal way? Copilot autocompletes style() but neither Element nor HtmlElement have that field. Once again the documentation comes to my rescue where Copilot failed.

I need the CssStyleDeclaration crate in order for this to work.

I can't really say I understand why the web assembly API is fragmented like this. I don't know if it's a Rust problem or not.

I need to turn a number like 3 into a string like 3px. Is format! really necessary for this? I guess I'll make a simple function since I'll use this multiple times.

While trying to fix this I realized using the Element type will force me to do a lot of casting on utility functions so I changed my Page class to use dyn_into<HtmlElement> when the divs are created. Then I had to update everything to use a HtmlElement instead of Element. Now it should work.

pub struct Styling<'a> {
    target: &'a HtmlElement
}

impl<'a> Styling<'a> {
    pub fn new(target: &'a HtmlElement) -> Self {
        Self { target }
    }

    pub fn set_px(&self, property: &str, value: f32) -> Result<(), wasm_bindgen::JsValue> {
        let style = self.target.style();
        style.set_property(property, &to_px(value))?;
        Ok(())
    }

    pub fn set_top_left(&self, top_left: &Point) -> Result<(), wasm_bindgen::JsValue> {
        self.set_px("left", top_left.x)?;
        self.set_px("top", top_left.y)?;
        Ok(())
    }

    pub fn set_size(&self, size: &Point) -> Result<(), wasm_bindgen::JsValue> {
        self.set_px("width", size.x)?;
        self.set_px("height", size.y)?;
        Ok(())
    }

    pub fn set_rect(&self, rect: &Rect) -> Result<(), wasm_bindgen::JsValue> {
        self.set_top_left(&rect.top_left)?;
        self.set_size(&rect.size)?;
        Ok(())
    }
}

pub fn to_px(value: f32) -> String {
    format!("{}px", value)
}

Copilot helped me write this. It's really good at repetitive stuff like this.

It seems that in Rust if you want to have a reference in a struct you need to use &'a for some reason. I'm not sure I understand exactly why, but it tells the compiler that the borrowed value will outlive the struct. The reason I don't understand this is that it's called "borrowing," so I can't understand what other possibility would there be. How would a struct outlive a borrowed value?

pub fn refresh_field(&self) -> Result<(), wasm_bindgen::JsValue> {
    Styling::new(&self.field_div)
        .set_rect(&self.field)
}

With our new styling class we can simply do this to update all the styling. This code is a bit repetitive to write so for the first time ever I've decided to rely on Copilot to write it for me. Just type pub fn refresh and wait for the LLM to write the rest. It also generated code for the score counters, which don't need Styling, but I'll have to write similar code for them anyway so this saves time.

Wait a second, now that I read the code, it's actually right. self.left_score_div.set_inner_text(&self.left_score.to_string());. Last time it used set_text_content(Some, I guess it was because the type was Element instead of HtmlElement?

let page = Page::from_global().expect("Failed to get window, document, or body element.");

let pong = Pong::new(&page, 0.0, 0.0, 640.0, 480.0).expect("Failed to create Pong");
pong.refresh_field();
pong.refresh_ball();
pong.refresh_left_pad();
pong.refresh_right_pad();
pong.refresh_left_score();
pong.refresh_right_score();
pong.refresh_speed_counter();

Perfect.

Rc and RefCell

The next problem is what to do with this:

let pong_rc = Rc::new(RefCell::new(pong));
let pong_clone = Rc::clone(&pong_rc);

Is this needed for the closures? What does it even do?

You have to enable multiple ownership explicitly by using the Rust type Rc<T>, which is an abbreviation for reference counting. The Rc<T> type keeps track of the number of references to a value to determine whether or not the value is still in use. If there are zero references to a value, the value can be cleaned up without any references becoming invalid.

https://doc.rust-lang.org/book/ch15-04-rc.html (accessed 2025-04-14)

Oh, so Rc<T> is just a shared pointer in Rust, like std::shared_ptr in C++.

"Cloning an Rc Increases the Reference Count". Wow. I know how shared pointers work but I would never, ever, ever in my life have guessed that Rc::clone is how you share the pointer. That's just not what that sounds like at all. That sounds like it creates a separate copy of a smart pointer such that the number of references isn't incremented. In C++ copying is done implicit and the counter changes without you having to type anything.

I'd like to move to RefCell but why is it even called Rc again? "an abbreviation for reference counting"?: Of reference counting? In the continuous aspect? Not even reference counter? Okay.

A common way to use RefCell<T> is in combination with Rc<T>. Recall that Rc<T> lets you have multiple owners of some data, but it only gives immutable access to that data. If you have an Rc<T> that holds a RefCell<T>, you can get a value that can have multiple owners and that you can mutate!

https://doc.rust-lang.org/book/ch15-05-interior-mutability.html#having-multiple-owners-of-mutable-data-by-combining-rct-and-refcellt (accessed 2025-04-14)

I think I understand how Rc and RefCell. In Rust, the compiler won't let you have two mutable references to the same object at the same time. This can be enforced simply by checking what functions are called. The problem I ran into is that I have event handlers.

The way the project is structured so far is that Javascript calls start_pong in Rust, and start_pong registers callbacks to mousemove and setInterval. This means that when start_pong returns, Pong must not be destroyed.

The lifetime of Pong must be long enough to survive all calls to the event handlers, which means that Pong can't be destroyed right after the web assembly entry point is called. To make it survive, using a smart pointer is probably the only way in Rust, so the use of Rc is correct and unavoidable. At least I understand why now.

RefCell allows the closure code to mutate a the shared Pong inside Rc. It enforces Rust's compile time rules at runtime, namely that you can't have two mutable references to the same object at the same time. This could happen, for example, if two threads tried to modify the same object. Fortunately the way event loops work is that two event handlers will never be called in parallel, as events are always processed serially, so this should never happen in my program.

let pong_rc = Rc::new(RefCell::new(pong));
{
    // Add mousemove event listener
    let pong_clone = Rc::clone(&pong_rc);
    let closure = Closure::wrap(Box::new(move |event: MouseEvent| {
        let mut pong = pong_clone.borrow_mut();
        pong.handle_mouse_move(&event);
    }) as Box<dyn FnMut(_)>);

    page.window.add_event_listener_with_callback(
        "mousemove",
        closure.as_ref().unchecked_ref()
    ).expect("should add mousemove event listener");

    closure.forget(); // Prevent the closure from being dropped
};

I think that's it?

While I was moving the code I noticed I don't need to return Result<(), wasm_bindgen::JsValue> everywhere, I can stop the chain by using let _ = to discard the returned result.

Next I do the same thing for the other closure.

This works but seeing as this code is really complicated I should check whether Copilot wrote it correctly. Do I need Box? as_ref().unchecked_ref()?

So it looks like a Box is a pointer. I'm not even being ironic. It's used to store data in the heap. That's what a pointer is for.

It seems forget is a bit tricky to use.

But fortunately that's not my case currently, so I don't have to worry about it. It seems the code that Copilot generated is correct, it's just that Rust is weird.

Refactoring Game Code

Now most code is moved from lib.rs to pong.rs. It's time the long mess it is.

One trick I learned is that you can do this is Rust:

let rubberbanded_ai_speed = {
    if player_skill > 0 {
        AI_SPEED * (1.0 + 0.25 * player_skill as f32)
    } else if player_skill < 0 {
        AI_SPEED / (1.0 + 0.25 * player_skill as f32 * -1.0)
    } else {
        AI_SPEED
    }
};

It's like a ternary operator but cooler.

Bounds Checking

We need some generic methods for checking boundaries, so we can make our if's smaller in the game loop.

pub fn keep_inside(&mut self, container: &Rect) {
    if self.y() < container.y() {
        self.set_y(container.y());
    }

    if self.x() < container.x() {
        self.set_x(container.x());
    }

    if self.right() > container.right() {
        self.set_right(container.right());
    }

    if self.bottom() > container.bottom() {
        self.set_bottom(container.bottom());
    }
}

Okay, turns out I lied and we do have getters and setters. Copilot makes it too easy to create them. I just tab and poof 6 getters and 6 setters created out of thin air.

No Clones

let ball_speed = self.ball.speed.clone();

Do I need this clone? I think I know what to do now that I have read.

I'll just add Copy and Clone to derive of Point so it can be trivially copied by the compiler.

After creating methods in Point I don't even need any of this.

let speed = self.ball.speed.length();
let mut normalized = self.ball.speed.normalized();

Tick Class

Having a game class is nice but there are many variables we need to keep track of each time the main game loop runs. It's easier to keep track of them if we create a separate class for that.

pub fn handle_game_tick(&mut self, delta: f32) {
    let mut tick = PongTick::new(self, delta);
    tick.process();
}

pub struct PongTick<'a> {
    pong: &'a mut Pong,
    delta: f32,
    rubberbanded_ai_speed: f32,
    speed_boost: f32,
    normalized: Point,
    scored: i32,
}

Doing all the physics is much easier now that I have getters for right and bottom.

pub fn compute_physics(&mut self) {
    let normalized = &mut self.normalized;
    let scored = &mut self.scored;
    let ball = &mut self.pong.ball;
    let left_pad = &self.pong.left_pad;
    let right_pad = &self.pong.right_pad;

    if normalized.y < 0.0 {
        if ball.rect.y() < 0.0{
            ball.rect.set_y(0.0);
            normalized.y *= -1.0;
        }
    }

    if normalized.x < 0.0 {
        if ball.rect.x() < 0.0 {
            ball.rect.set_x(0.0);
            normalized.x *= -1.0;
            *scored = -1;
        }

        if ball.rect.intersects_with(&left_pad.rect) {
            normalized.x *= -1.0;
            self.pong.speed_counter += 1;
        }
    }

    if normalized.y > 0.0 {
        if ball.rect.bottom() > self.pong.field.bottom() {
            ball.rect.set_bottom(self.pong.field.bottom());
            normalized.y *= -1.0;
        }
    }
    
    if normalized.x > 0.0 {
        if ball.rect.right() > self.pong.field.right() {
            ball.rect.set_right(self.pong.field.right());
            normalized.x *= -1.0;
            *scored = 1;
        }

        if ball.rect.intersects_with(&right_pad.rect) {
            normalized.x *= -1.0;
            self.pong.speed_counter += 1;
        }
    }
}

It seems I can alias variables with &. To set a variable like scored, I need &mut self.scored to create a mutable reference, and then *scored = -1 to deference it before setting it. This makes sense since in this context score = foo without dereference would allow us to change score to a different &mut. The asterisk is new, however.

Testing the Code

I've finished rewriting everything, but there is just one small problem: the code doesn't work. Most of it is working, but for some reason moving my mouse moves the right pad instead of the left pad and collision detection isn't working at all. Since I changed a lot of code I can't really tell what part introduced the bug.

This sounds like a good time to start writing tests to check if the code is working so I can find what part is bugged without having to rebuild this and reload the webpage in the browser every single time.

Testing Point

It seems Rust has first-party support for writing unit tests just like Zig. Amazing. Let's start by testing our most primitive classes. They stand at the bottom of the project jenga tower, with no potentially broken dependencies, so we can easily test that the methods work without having to worry about whether code elsewhere is correct or not.

I recall that I haven't tested Copilot's test-writing capabilities during my first experience of it, so for the sake of the experiement, let's ask Copilot to do it just to see what happens.

@workspace /tests generate tests for all of Point's methods

Copilot did generate what appears to be 150 lines of test code, but it also deleted my impl Mul<f32> for Point and similar operators that weren't inside the struct for some reason. I can't use Apply in Editor when it just deleted all my code like that so I guess I'll copy and paste the code manually.

"point::Point doesn't implement Debug"? It seems I don't have an equality operator either for my Point either.

All public types implement Debug (C-DEBUG)

https://rust-lang.github.io/api-guidelines/debuggability.html (accessed 2025-04-13)

Okay, I guess we need a Debug trait for point on top of Clone AND Copy. And for a lot of similar things as well. I thought they were optional the first time I had to use Clone but now I'm going to assume they're practically mandatory to program in Rust.

And I need PartialEq to make == work...

#[derive(Copy, Clone, Debug, PartialEq)]

I'm glad Copilot generated this for me because I never remember it, but on the other hand I'm not sure this test is written right.

#[test]
fn test_length() {
    let point = Point::new(3.0, 4.0);
    assert_eq!(point.length(), 5.0);
}

Doing == with floating point arithmetic feels wrong. It does pass the test, though, so I guess I'll just leave it like that. Actually, better not since the other tests also feel impossible to understand. I'll just rewrite it to use the actual formula.

let expects = f32::sqrt(3.0 * 3.0 + 4.0 * 4.0);
assert_eq!(point.length(), expects);

Now if something (like Copilot) ever changes length() by accident we will know it.

fn test_normalized() {
    let point = Point::new(3.0, 4.0);
    let normalized = point.normalized();
    assert!((normalized.x - 0.6).abs() < 1e-6);
    assert!((normalized.y - 0.8).abs() < 1e-6);

    let zero_point = Point::new(0.0, 0.0);
    assert_eq!(zero_point.normalized(), Point::new(0.0, 0.0));
}

I know this is probably right but I can't read this Copilot. Also if you know to use error margins like that why didn't you do it with length?

#[test]
fn test_almost_equal() {
    let margin = 1e-6;
    let p = Point::new(3.0, 4.0); 
    let p2 = p + Point::new(margin / 2.0, -margin / 2.0);
    assert!(p.almost_equal(&p2));
}

#[test]
fn test_normalized() {
    let point = Point::new(3.0, 4.0);
    let length = point.length();
    let expected = Point::new(point.x / length, point.y / length);
    let normalized = point.normalized();
    assert!(normalized.almost_equal(&expected));
    
    let zero_point = Point::new(0.0, 0.0);
    assert_point_xy(&zero_point.normalized(), 0.0, 0.0);
}

My Point tests all pass. Nothing needed to be fixed so nothing was broken. The bug is elsewhere.

Testing Rect

While coding the tests for Rect I learned you can create your own custom methods to use in string formatting like this:

impl fmt::Display for Rect {
    fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result {
        write!(
            f,
            "[{}, {}, {}, {}]",
            self.x(),
            self.y(),
            self.width(),
            self.height()
        )
    }
}

#[test]
fn test_intersects_with() {
    let tl = Rect::with_xywh(0.0, 0.0, 1.0, 1.0);
    let tr = Rect::with_xywh(2.0, 0.0, 1.0, 1.0);
    let bl = Rect::with_xywh(0.0, 2.0, 1.0, 1.0);
    let br = Rect::with_xywh(2.0, 2.0, 1.0, 1.0);

    let rects = [tl, tr, bl, br];
    for i in 1..rects.len() {
        for j in 0..i {
            assert!(
                !rects[i].intersects_with(&rects[j]),
                "Rectangles should not intersect: {} and {}",
                rects[i], rects[j]
            );
        }
    }

The tests pass so the bug must be elsewhere.

I don't know how to debug web assembly so we really have to make these tests work to find the bugs.

Copilot has proven to be very convenient for writing tests' general structure and it really amazes me how it's able to get the math right every time. I don't know if this is a special ability of Copilot's programming or simply a fortunate side effect of there being billions of re-implementation of basic Point/Vec2 logic on Github.

One shortcoming of Copilot I keep noticing and that has become very clear now is that it simply can't generate reusable code. Whenever you tell it to generate something, it will generate the functions you asked for and will fill those functions with code, but it can extract repetitive portions of its own code and put that in an utility function.

For example, the generated tests for Rect have a separate assert! for each variable of the Rect, x(), y(), width(), and height(). It was trivial to write an assert_rect_xywh to let me use a single line of code instead of four every time.

I've read there are all sorts of AI code assistants, both local and as a service, so perhaps some of them are better in this reusability regard.

Since the bug isn't here, it must be in higher level components, and that complicates things.

Testing Pong

Testing Pong isn't really possible without mocks because it depends on Page and Page depends on web_sys::Window. Without a way to mock a Page or a Window, we can't instantiate a valid Pong. On the other hand, if we start mocking things it will just never ends because then every time we write a test it must have the mock or we can't instantiate anything.

Fortunately we can get around this by separating our game logic from the code that interacts with external dependencies. Ideally we want Pong to handle sending graphic updates to the frontend, and some intermediary class with no div dependencies for the game logic that we can trivially instantiate and modify in tests.

pub struct Pong {
    pub field_div: HtmlElement,
    pub ball_div: HtmlElement,
    pub left_pad_div: HtmlElement,
    pub right_pad_div: HtmlElement,
    pub left_score_div: HtmlElement,
    pub right_score_div: HtmlElement,
    pub speed_counter_div: HtmlElement,
    pub logic: PongLogic,
}

pub struct PongLogic {
    pub field: Rect,
    pub ball: Ball,
    pub left_pad: Pad,
    pub right_pad: Pad,
    pub left_score: u32,
    pub right_score: u32,
    pub speed_counter: u32,
}

This may look like it's needlessly complicated, after all, a lot of game engines like Godot just let you put graphics, audio, and physics in a single node in a scene. Why can't we just do that? We could do that without dependencies if we had a proper protocol to send messages from the game objects to specific components without depending on them for instantiation. That would be more complicated to implement given we have only 3 objects in this entire game, so for now this should be enough to follow the single responsibility principle.

An example of the separation of concerns can be seen in this event handler:

pub fn handle_mouse_move(&mut self, event: &MouseEvent) {
    let cursor = self.mouse_point_to_field(&event);
    self.logic.handle_mouse_move(cursor);
}

The logic class doesn't have access to any of the div's, so in order for it to handle the mouse cursor we must translate its coordinates to a system that logic has access to. In this case, the external Pong class translates the mouse cursor's coordinates to coordinates relative to the field_div. To logic, the mouse cursor position is always relative to its field's top-left corner, and it doesn't even know that in the webpage the field is centered on the page thanks to CSS.

One negative side of this is that because we don't want any code that deals with graphics in our game logic, that means the simplest way to update the graphics is to just update everything every time even when it hasn't changed.

let closure = Closure::wrap(Box::new(move || {
    let mut pong = pong_clone.borrow_mut();
    pong.logic.handle_game_tick(DELTA);

    pong.refresh_field();
    pong.refresh_left_pad();
    pong.refresh_right_pad();
    pong.refresh_ball();
    pong.refresh_left_score();
    pong.refresh_right_score();
    pong.refresh_speed_counter();
}) as Box<dyn Fn()>);

We could fix this with some is_dirty field on the game objects that we could check during graphics rendering, but it's not really necessary unless performance becomes an issue. For now I'll just move the refresh code to requestAnimationFrame to minimize the effects of layout changes.

pub fn on_request_animation_frame(&self, callback: Box<dyn Fn() -> bool>) {
    let request_animation_frame = Rc::new(RefCell::new(None as Option<Box<dyn Fn()> >));
    let inner_ref = request_animation_frame.clone();

    let wrapper_cb = Closure::wrap(Box::new(move || {
        let should_continue = callback();
        if should_continue {
            inner_ref.borrow().as_ref().unwrap()();
        }
    }) as Box<dyn Fn()>);

    *request_animation_frame.borrow_mut() = {
        let window = self.window.clone();

        Some(Box::new(move || {
            window.request_animation_frame(
                wrapper_cb.as_ref().unchecked_ref(),
            ).unwrap();
        }))
    };

    request_animation_frame.borrow().as_ref().unwrap()();
}

let animation_frame_handler = {
    let pong_clone = pong_rc.clone();

    Box::new(move || {
        let pong = pong_clone.borrow();
        pong.refresh_field();
        pong.refresh_left_pad();
        pong.refresh_right_pad();
        pong.refresh_ball();
        pong.refresh_left_score();
        pong.refresh_right_score();
        pong.refresh_speed_counter();

        true
    })
};

page.on_request_animation_frame(animation_frame_handler);

I don't really understand this. It works, but why do I need as_ref()? What does unchecked_ref do? At least I think I understand closures and Box'ing them now.

One extremely confusing thing about Web Assembly is that there are closures in Rust and Closures in the Web Assembly API that are used to pass closures to Javascript.

When you write Box<move || {...}> that is a closure in Rust that captures variables. Note that the type for this is just Box<>, which as we know is just a weird name for a pointer to the heap. The code Box<dyn Fn() -> bool> specifies a Rust closure that returns a boolean.

Meanwhile the Closure::wrap and Closure::new functions are from Web Assembly.

To use a variable both inside and outside a Rust closure, I need Rc. In this case I need to call request_animation_frame both from inside its handler and from outside the the time it's called.

You need as_ref().as_unchecked_ref() to pass a Javascript closure to a Javascript function.

On the other hand, .borrow() borrows a reference (&), or rather, as what I assume Rust calls a "borrowing" or something like that. You can't call unwrap() on a borrowed Option, so you have to call as_ref to de-borrow it in order to call unwrap. Honestly, I have no idea what is going on here. The only thing I understood is that this as_ref isn't even the same as_ref as the other as_ref because this one is from &Option<T>, not for Closure! It just happens to have the same name!

Oh I get why you can redeclare variables in Rust now.

{
    const FRAME_RATE_IN_MS: i32 = 50;
    const DELTA: f32 = 1.0 / FRAME_RATE_IN_MS as f32;

    let pong_rc = pong_rc.clone();
    page.on_interval(FRAME_RATE_IN_MS, Box::new(move || {
        let mut pong = pong_rc.borrow_mut();
        pong.logic.handle_game_tick(DELTA);

        true
    }));
}

Instead of coming up with a new name for the same variable every time I clone or borrow it I can just re-use the same name. Above, let pong_rc = pong_rc.clone() defines a pong_rc in an internal scope using the pong_rc of an external scope.

Redeclaring variables feels disgusting but the borrow checker is too annoying for me to deal with this in any other way.

pub fn on_interval(&self, interval: i32, callback: Box<dyn Fn() -> bool>) {
    let window = self.window.clone();

    struct RegisteredInterval {
        interval_handle: i32,
        closure: Closure<dyn Fn() -> ()>
    };
    
    let reg = Rc::new(RefCell::new(None as Option<RegisteredInterval>));

    let js_closure = {
        let window = self.window.clone();
        let reg = reg.clone();

        Closure::wrap(Box::new(move || {
            let should_continue = callback();
            if !should_continue {
                window.clear_interval_with_handle(reg.borrow().as_ref().unwrap().interval_handle);
            }
        }) as Box<dyn Fn()>)
    };
    
    let handle = window.set_interval_with_callback_and_timeout_and_arguments_0(
        js_closure.as_ref().unchecked_ref(),
        interval,
    ).unwrap();

    *reg.borrow_mut() = Some(RegisteredInterval {
        interval_handle: handle,
        closure: js_closure
    });
}

It's taking way too long to program even the simplest things because the borrow checker doesn't let me do anything. I need to keep an unused closure field in RegisteredInterval to keep it from being dropped, because if it's dropped on the Rust side Javascript will automatically remove the setInterval handler. That doesn't even make sense but it happened.

I assume this means Rust has destructors, unlike Zig.

While refactoring I found where my bug was so I guess I could stop fixing the code now. However there is a few places we can still improve and few Rust features we can still learn, so let's do that.

Macros

In my Point struct I have code like this that I don't like:

impl Mul<f32> for Point {
    fn mul(self, rhs: f32) -> Self::Output {
        Point::new(self.x * rhs, self.y * rhs)
    }
    
    type Output = Point;
}

impl Mul for Point {
    fn mul(self, rhs: Point) -> Self::Output {
        Point::new(self.x * rhs.x, self.y * rhs.y)
    }
    
    type Output = Point;
}

impl MulAssign<f32> for Point {
    fn mul_assign(&mut self, rhs: f32) {
        self.x *= rhs;
        self.y *= rhs;
    }
}

Surely there must be a better way than writing the same thing 16 times and then tests for it 16 times? If this was Zig it would be trivial to write a compile-time for loop to generate all this code. How can we do this in Rust?

To be honest, I'm not sure if the answer is macros or not. It seems macros are Rust's templates, but even in C++ I wouldn't be able to do this metaprogrammatically because I'm just not good with templates in general. I suppose we can ask Copilot to do it. I'd rather not but metaprogramming is just very tricky to learn in general.

@workspace /fix write a single macro to generate the add, sub, mul, and div implementation of the Point struct to replace the existing repetitive code.

Here is what Copilot gave me:

macro_rules! impl_point_ops {
    ($trait:ident, $method:ident, $op:tt) => {
        impl $trait for Point {
            type Output = Point;

            fn $method(self, rhs: Point) -> Self::Output {
                Point::new(self.x $op rhs.x, self.y $op rhs.y)
            }
        }

        impl $trait<f32> for Point {
            type Output = Point;

            fn $method(self, rhs: f32) -> Self::Output {
                Point::new(self.x $op rhs, self.y $op rhs)
            }
        }
    };
}

I don't understand what is this or how to use it, but it does seem to do what I want. Presumably I'm supposed to impl_point_ops("Add", "add", "+")? I don't see any for loops. I guess it's time to consult the book.

It's actually called macro_rules!. Love the enthusiasm.

The book even has an example for Vec with a macro_rules! for operators that we can use.

That's cool but do I really have to implement both add and add_assign? Even with macros I'd expect there to be some way to just skip doing that since a = a + b and a += b should always be literally the same thing.

I can't find the answer on Google.

Is there a trait I can derive to skip having to implement both Add and AddAssign myself?

Copilot: "No, there isn't a built-in trait in Rust that allows you to derive both Add and AddAssign automatically. These traits must be implemented manually or through macros if you want to customize their behavior. However, you can simplify your code by using a macro to generate both implementations together."

Really?

Oh well, at least there are macros.

impl_point_ops!(Add, add, +);
impl_point_ops!(Sub, sub, -);
impl_point_ops!(Mul, mul, *);
impl_point_ops!(Div, div, /);
impl_point_ops_assign!(AddAssign, add_assign, +=);
impl_point_ops_assign!(SubAssign, sub_assign, -=);
impl_point_ops_assign!(MulAssign, mul_assign, *=);
impl_point_ops_assign!(DivAssign, div_assign, /=);

Reportable Collisions

The last thing I'd like to do is refactor the physics and AI logic to decouple the analysis of the world from its consequences. Ideally, instead of the same code checking for collisions and then changing the speed of the ball, the only thing it should do is check for collisions and return a list of collisions found. A separate subprogram would be responsible for taking these results and acting on it.

Separating the code like this will let us write tests for the physics loop. We'll be able to test if, when the ball is at a certain position with a certain speed, a collision is found or not, and what would be the consequence of it.

Let's start with the AI because it's simpler.

pub struct EnemyStrategy {
    pub movement: f32,
}

impl EnemyStrategy {
    pub fn new(pad: &Pad, ball: &Ball, max_movement: f32) -> Self {
        let pad_center = pad.rect.center().y;
        let ball_center = ball.rect.center().y;
        
        let dist = (ball_center - pad_center).abs();
        let min_dist = max_movement.min(dist);

        if pad_center < ball_center {
            EnemyStrategy {
                movement: min_dist,
            }
        } else {
            EnemyStrategy {
                movement: min_dist * -1.0,
            }
        }
    }
}

impl<'a> PongTick<'a> {
    pub fn compute_enemy_ai(&mut self, alpha: f32) {
        let max_movement = self.rubberbanded_ai_speed * self.delta * alpha;
        let strategy = EnemyStrategy::new(&self.logic.right_pad, &self.logic.ball, max_movement);

        // apply strategy
        self.logic.right_pad.rect.top_left.y += strategy.movement;
        
        // clamp the right pad position to the field
        self.logic.right_pad.rect.keep_inside(self.logic.field);
    }
}

#[cfg(test)]
mod tests {
    use super::*;

    #[test]
    pub fn test_enemy_strategy() {
        let max_movement = 5.0;
        
        let pad = Pad {
            rect: Rect::with_xywh(0.0, 100.0, 10.0, 50.0),
        };
        
        let mut ball = Ball {
            rect: Rect::with_xywh(0.0, 0.0, 10.0, 10.0),
            speed: Point::new(10.0, 10.0),
        };
        
        {
            let strategy = EnemyStrategy::new(&pad, &ball, max_movement);
            assert!(strategy.movement < 0.0);
        }

        ball.rect.set_y(200.0);
        {
            let strategy = EnemyStrategy::new(&pad, &ball, max_movement);
            assert!(strategy.movement > 0.0);
        }

        ball.rect.set_y(-ball.rect.height() / 2.0 + pad.rect.center().y);
        {
            let strategy = EnemyStrategy::new(&pad, &ball, max_movement);
            assert!(strategy.movement == 0.0);
        }
    }
}

Perfect.

Now we just have to do the same thing for the collision system.


pub struct CollisionResults {
    pub ball_position: Point,
    pub ball_speed: Point,
    pub pad_hits: u32,
    pub scored: i32,
}

impl CollisionResults {
    pub fn new(ball: &Ball, ball_speed: &Point, field: &Rect, left_pad: &Pad, right_pad: &Pad) -> Self {
        let mut ball_rect = ball.rect;
        let mut ball_speed = *ball_speed;
        let mut pad_hits = 0;
        let mut scored = 0;

        if ball_speed.y < 0.0 {
            if ball_rect.y() < field.y() {
                ball_rect.set_y(0.0);
                ball_speed.y *= -1.0;
            }
        }

        if ball_speed.x < 0.0 {
            if ball_rect.x() < 0.0 {
                ball_rect.set_x(0.0);
                ball_speed.x *= -1.0;
                scored = -1;
            }

            if ball_rect.intersects_with(&left_pad.rect) {
                ball_speed.x *= -1.0;
                pad_hits += 1;
            }
        }

        if ball_speed.y > 0.0 {
            if ball.rect.bottom() > field.bottom() {
                ball_rect.set_bottom(field.bottom());
                ball_speed.y *= -1.0;
            }
        }
        
        if ball_speed.x > 0.0 {
            if ball.rect.right() > field.right() {
                ball_rect.set_right(field.right());
                ball_speed.x *= -1.0;
                scored = 1;
            }

            if ball.rect.intersects_with(&right_pad.rect) {
                ball_speed.x *= -1.0;
                pad_hits += 1;
            }
        }

        CollisionResults { ball_speed, ball_position: ball_rect.top_left, pad_hits, scored }
    }
}

#[cfg(test)]
mod tests {
    use super::*;

    #[test]
    pub fn test_collision_results() {
        let field = Rect::with_xywh(0.0, 0.0, 640.0, 480.0);
        let left_pad = Pad::new(Rect::with_xywh(10.0, 0.0, 10.0, 50.0));
        let right_pad = Pad::new(Rect::with_xywh(620.0, 0.0, 10.0, 50.0));
        let mut ball = Ball::new(Rect::with_xywh(300.0, 200.0, 10.0, 10.0), Point::new(5.0, 5.0));
        let mut ball_speed = Point::new(5.0, 5.0);

        // test no collision
        {
            let collision_results = CollisionResults::new(&ball, &ball_speed, &field, &left_pad, &right_pad);
            assert_eq!(collision_results.pad_hits, 0);
            assert_eq!(collision_results.scored, 0);
            assert_eq!(collision_results.ball_position, ball.rect.top_left);
            assert_eq!(collision_results.ball_speed, ball_speed);
        }

        // left colllision...
        // right colllision...
        // left score...
        // right score...
        // top collision...
        // bottom collision...
    }
}

It's a bit more complex than I'd hoped but it lets us test it and that's what matters.

Now if we wanted to add something fancier like changing the angle of the ball based on how far from the center of the pad it hit, it would be very easy to test that the math is working without having to reload the game a hundred times and wait until the ball hits every angle.

With this new low-level structure we can simplify our high level code to just applying the effects:

pub fn compute_physics(&mut self) {
    let results = CollisionResults::new(
        &self.logic.ball,
        &self.normalized_ball_speed,
        &self.logic.field,
        &self.logic.left_pad,
        &self.logic.right_pad
    );

    self.logic.ball.rect.top_left = results.ball_position;
    self.normalized_ball_speed = results.ball_speed;

    if results.pad_hits > 0 {
        self.logic.speed_counter += results.pad_hits;
    }

    if results.scored != 0 {
        self.scored += results.scored;
    }
}

Conclusions

I have a much better grasp of Rust now that I have read. I still don't fully understand advanced concepts like borrowing, but I assume it will only be a problem when dealing with closures. If I did the event handling in Javascript instead I assume it would have been a lot simpler and I wouldn't have had as much trouble.

There are lots of things we could do this this project but none of them will require learning anything special. It's a game project, we're only going to have game development problems from now on, like how to make a unique game out of pong, how to make it entertaining.

Statistics

I've used Copilot a lot these last two days, and much of it unconsciously. When I made the project with my free account all the way to publishing it on Github, my quotas were at 6% for chat requests and 11% for code completion. Currently, I see they are at 11% for chat requests in 66% for code completion. This means that the free tier of Copilot wouldn't even last a whole week at this rate.

Again, I'm really not sure what counts as a "code completion" because in many cases the completion was a semi colon or the name of a variable I was about to type anyway. I'm not even sure if Copilot provides any benefit by interrupting me to tell me what I'm about to write.

I think I can use it once per month to learn a new language. Maybe I'll learn Go next.

Commit

Copilot, what did I do?

"add Pad and Page structures; enhance styling and HTML navigation; update README for clarity"

No. I "fixed code," Copilot.

Written by Noel Santos.

About the Author

I'm a self-taught Brazilian programmer graduated in IT from a FATEC. In a world of increasingly complex and essential computers, I decided to use my technical expertise in hardware, desktop applications, and web technologies to create an informative resource to make PC's easier to understand.

View Comments