Update base composition with base(), base_mut() methods.
#21
Conversation
|
Oops didn't lint, will fix :^) |
Lemiczek
force-pushed
the
base_reentrancy
branch
from
8b942df
to
9c3ef55
Compare
src/intro/hello-world.md
Outdated
| 5. The `#[base]` attribute declares the `sprite` field, which allows `self` to access the base instance (via `base()` or `base_mut()` as Rust does | ||
| not have native inheritance). |
There was a problem hiding this comment.
The old sentence is still true -- the point is that since no inheritance is possible, we use composition. The base()/base_mut() are just accessors.
It still makes sense to mention those, but it should be a new sentence and not replace the previous one.
src/intro/hello-world.md
Outdated
| // or verbose: | ||
| // self.sprite.set_position( | ||
| // self.sprite.position() + velocity * delta as f32 | ||
| // self.base_mut().set_position( | ||
| // self.base().position() + velocity * delta as f32 | ||
| // ); |
There was a problem hiding this comment.
I don't think this compiles -- there is a &mut borrow from base_mut() pending when base() is invoked.
Maybe change it to use two separate statements, storing the position in a variable.
There was a problem hiding this comment.
I just ran into this while going through the book myself, and you're exactly right, it doesn't compile. Here's what I have, which does compile and functions correctly:
let rotation = (self.angular_speed * delta) as f32;
self.base_mut().rotate(rotation);
let rotation = self.base().get_rotation();
let velocity = Vector2::UP.rotated(rotation) * self.speed as f32;
self.base_mut().translate(velocity * delta as f32)
Bromeon
force-pushed
the
base_reentrancy
branch
from
14baec8
to
54b1c89
Compare
|
Sorry, I must merge this, there have already been 3 (!) people stumbling upon the outdated comment just in these few days. For the future, we should probably open book PRs alongside library PRs, or at least follow up within a day or so 🙂 |
|
Thank you very much for getting this through @Bromeon ! A suggestion of mine to prevent such thing in the future would be to have the Happy to help on that contribution if you feel this is worthwhile. |
|
In this context, git tags seem like an extra maintenance burden without providing useful information to the user (as they don't follow SemVer). It's basically using the Git SHA with extra manual steps. That said, things might be a bit clearer once the library is on crates.io, and we can let the book explicitly target a given version. |
|
By git tags, I meant SemVer versioning that matches your crates.io release. (which I get that don't exist yet) All of these should allign. My suggestion was one for the bigger picture of overall release management. |
|
That definitely makes sense and is also what we have been doing for gdnative 👍 |
Related to: godot-rust/gdext#501
Reflects changes made by changing the way we access our
Base<T>, essentially replacing our usage ofself.spriteto instead utilizebase()andbase_mut().There may be more places where information may be slightly inaccurate due to the PR. I looked over the book, and applied changes where it made sense. That said, I could've missed something, if so, I can amend. 👍