PlacementManager CurrentEraserMouseCoordinates fix #5576
Merged
+3
−2
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
…the wrong coordinates
|
the day I finally understand the coordinate systems will be the day I implement copy-paste (and completely overhaul PlacementManager) |
It's not that complicated, but placement manager is really not the place to learn, its shitty, partially ancient code that no one has properly cleaned up in a long time. Point in case, the bug here was only possible because placement manager is hot garbage and was and is still assuming that the entity coordinates are relative to the map. |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Resolves #5575
With the recent modification to this method to remove deprecated calls, it appears a bug was introduced which messed up the coordinates being used in
PlacementManager'sCurrentEraserMouseCoordinates. It appears the issue has something to do with how the call to theToCoordinatesmethod includes an entity which causes it to seemingly convert the coordinates to be relative to the entity, which isn't very conducive to the erase method here. One day I hope to understand the coordinate systems in this game so I can know if my read on this is correct, but today is not that day.I've resolved the issue by using a different method overload for
EntityCoordinates'sToCoordinateswhich doesn't have an argument for the entity.This seems to work rather well, however during testing I noticed a new bug, which threw an endless stream of error messages if I happened to mouse over any UI element. I've added an additional fix for this, which simply checks whether the MapId we're moused over is equal to the
MapId.Nullspacevalue. This bug is also present in the current version of the code, so I figured I'd resolve it as well.My fix for this simply causes the method to return its false result, which functionally just makes the selector freeze in place while you're mousing over UI elements. It doesn't feel the best, but I fail to understand the coordinate systems in this game, so I won't be resolving that.
FIXED.mp4