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

Made inbounds use map parameters #73503

Merged
merged 3 commits into from
May 7, 2024
Merged

Conversation

PatrikLundell
Copy link
Contributor

Summary

None

Purpose of change

Make 'inbounds' use the map parameters rather than hard coded overloaded operations using those same values.

Describe the solution

Changed the map::inbounds operations to check against the map's own parameters rather than hard coded limits that are the same as the map's at the time the code is written. This means tinymap can just call the map operations rather than overload them.

Removed confusing hard coded open cuboid checks against OVERMAP_HEIGHT +1 and replaced these with map::inbounds checks instead. If the reality bubble is expanded in the future, there's one less bug to find (looks like somebody really wanted to find a use for the cuboid stuff).

Removed a couple of weird usages of inbounds_z, where they really were used as "has the value changed from the initial value?" checks, which just is confusing.

Describe alternatives you've considered

Change inbounds_z to return false if the Z value isn't the map's Z value when Z levels aren't supported and use that in the inbounds checks. Reverted that because there might be some flaky tinymap using code that does stuff with caches anyway when the Z level is off (and it was a mistake to commit the changed code originally).

Testing

Load a save and walk around a bit. The changes should make no functional difference.

Additional context

@github-actions github-actions bot added Vehicles Vehicles, parts, mechanics & interactions [C++] Changes (can be) made in C++. Previously named `Code` json-styled JSON lint passed, label assigned by github actions astyled astyled PR, label is assigned by github actions labels May 4, 2024
@PatrikLundell PatrikLundell marked this pull request as draft May 4, 2024 12:43
@PatrikLundell
Copy link
Contributor Author

PatrikLundell commented May 4, 2024

Well, that was unexpected...

Testing blows up because the test code expects access to other Z levels to return the value of the current Z level for maps that don't support Z levels, i.e. that the Z level is ignored.

Another test (presumably: can't find the possible reference to a test case among all the error output) expects to get away with setting furniture at the current Z level while writing to a different one (again, it expects the inbounds test to ignore the Z level parameter, as well as the code itself further on).

Utterly disgusting, in my opinion.

I can't say if it's only testing (which can be changed) or if this disgusting use in also practiced somehow for some bizarre reason.

@PatrikLundell PatrikLundell marked this pull request as ready for review May 5, 2024 09:29
@github-actions github-actions bot added the BasicBuildPassed This PR builds correctly, label assigned by github actions label May 5, 2024
@Maleclypse Maleclypse merged commit 3bb1d34 into CleverRaven:master May 7, 2024
27 of 39 checks passed
@PatrikLundell PatrikLundell deleted the inbounds branch May 7, 2024 07:36
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
astyled astyled PR, label is assigned by github actions BasicBuildPassed This PR builds correctly, label assigned by github actions [C++] Changes (can be) made in C++. Previously named `Code` json-styled JSON lint passed, label assigned by github actions Vehicles Vehicles, parts, mechanics & interactions
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants