Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Improved RoomCoordinate and RoomXY, new RoomOffset for optimized arithmetic #543

Merged
merged 27 commits into from
Oct 29, 2024

Conversation

khoover
Copy link
Contributor

@khoover khoover commented Oct 9, 2024

Adds indexing impls for RoomCoordinate and a wrapper type that allows indexing nested arrays by RoomXY. Re-writes LocalCostMatrix and LocalRoomTerrain indexing in terms of the RoomCoordinate indexing impls.

Also adds a new RoomOffset type which can be used for better-optimized arithmetic, as well as being the output of coordinate-coordinate subtraction.

Can index into `[T; ROOM_SIZE]` to get `&(mut) T`, or `[T; ROOM_AREA]`
to get `&(mut) [T; ROOM_SIZE]`.
Introduces wrapper types that can be used to index by `RoomXY` directly,
instead of going coordinate-by-coordinate.

Also replaces `ROOM_SIZE as usize` with `ROOM_USIZE`, where used.
Implicitly allows direct access to `bits`, in the form of the public
`XMajor` member.
Removes unsafe block that directly converted to linear coordinates.
Copy link
Contributor

@jciskey jciskey left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM, not seeing any major incompatibilities or issues. Even the documentation comments are solid.

@khoover

This comment was marked as resolved.

@khoover

This comment was marked as off-topic.

@khoover khoover requested a review from jciskey October 14, 2024 04:11
@khoover khoover changed the title Improved indexing with RoomCoordinate and RoomXY Improved indexing and performance with RoomCoordinate and RoomXY Oct 14, 2024
@khoover khoover marked this pull request as draft October 14, 2024 15:57
For consistency's sake, return a `Result` from `RoomOffset::new` instead
of an `Option`. Also better document all the new methods.
@khoover khoover marked this pull request as ready for review October 29, 2024 06:40
@khoover
Copy link
Contributor Author

khoover commented Oct 29, 2024

What I'd want to do is replace the Sub impl for RoomXY with one that returns an OffsetXY or similar, instead of i8 pairs.

@khoover khoover changed the title Improved indexing and performance with RoomCoordinate and RoomXY Improved RoomCoordinate and RoomXY, new RoomOffset for optimized arithmetic Oct 29, 2024
@shanemadden shanemadden merged commit 5f1ea09 into rustyscreeps:main Oct 29, 2024
2 checks passed
@khoover khoover deleted the better-indexing branch November 9, 2024 16:55
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants