Clearly separate local and global directions when using Side#126
Open
dhouck wants to merge 3 commits intoNorth-Western-Development:1.20.1from
Open
Clearly separate local and global directions when using Side#126dhouck wants to merge 3 commits intoNorth-Western-Development:1.20.1from
dhouck wants to merge 3 commits intoNorth-Western-Development:1.20.1from
Conversation
Instead of having different identical variants with different names, use gson's SerializedName attribute to allow one variant to be referenced with all relevant names. Additionally, keep track of which of those names are relative vs. absolute, so eg. "north" and "back" are no longer synonyms. Incidentally, add more top/bottom as extra names for up/down. The simplification also applies to RobotOperationSide, but all variants of that are local so the global/local split does not. In theory it could apply to MovementDirection and RotationDirection, but those are serialized/deserialized as NBT ints so this would break deserialization. Backwards compatibility: Despite being in the api package, nobody outside this mod seems to use the class. Any VM code that addresses sides by index will be affected, but the index to side mapping was never intuitive or documented.
HorizontalBlockUtils used minecraft Direction as both global and local direction, and Side.getDirection was also used as both. This refactors all of HorizontalBlockUtils into Side, and more clearly separates global sides (minecraft Direction), local sides (integer indexes), and ambiguous (the Side type). Several bugs with incorrect or missing global/local conversion were also fixed. The RedstoneInterface block and card now both support either global or local sides in their RPC API, and will respond appropriately (ie. a call to set the NORTH value will set NORTH regardless of orientation, and a call to set the FRONT value will set the FRONT, which might be any cardinal direction). This also works for the bundled interface on the block. The ComputerBlockEntity no longer converts to local before getting capabilities. Any bus item that should expose computer block capabilities accepts a global Direction, and must respond as such; it should have access to the block state and can do the rotation on its own if necessary. Backwards compatibility: Any VM code that uses a global name in a context where it was interpreted as local, or vice versa, will break; I expect this to be rare and easy to fix. I think that's the only functionality change I wouldn't call a bugfix, but I did not thoroughly test the old behavior of Project Red: Transmission bundled output so something unexpected might have changed there. Further work: The redstone interface card is not up to feature parity with the block. Probably it would be best if they shared more code instead of duplicating most of the functionality for both, to help bring the card up to parity and avoid future issues.
Author
|
EDIT: I have now tested and confirmed this. Not just for the one commit I noticed earlier, with block detection, but also for the item drop placement.
|
Author
This file contains hidden or 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
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
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.
The
Sideenum can be used for either local directions based on the blockFACING, or global cardinal directions. Previously most uses of the enum got the two mixed up, leading to a variety of bugs and inconsistencies. This clearly separates when something is a globalDirection, a local direction, or something that could be either.I found this while working on improving the event support for the Redstone Interface block, following my work on #124. The actual event improvements are a separate pull request I am not quite done with, but this was a pre-requisite, because the events previously gave global directions, which could then not accurately be used in method calls.