Rename GTRegistry.remap() to .alias(), create a proper Remap, and make Aliases not break 1:1 key-value pairing#4934
Open
DilithiumThoride wants to merge 1 commit into
Open
Rename GTRegistry.remap() to .alias(), create a proper Remap, and make Aliases not break 1:1 key-value pairing#4934DilithiumThoride wants to merge 1 commit into
DilithiumThoride wants to merge 1 commit into
Conversation
…eak 1:1 key-value pairing.
DilithiumThoride
added
the
type: refactor
DilithiumThoride
added
type: bugfix
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.
What
GTRegistry.remap()does not actually change key-value mappings. It creates a second key that maps to a single value, one-directionally. This breaks the 1:1 Key-Value pairing assumption present in the other registry functions, and (among other things) causes.remove()to break again.Implementation Details
I do not like this but it feels necessary.
Created an additional
Map<K, K>, where the first key is an Alias and the second key is a real key within the main KeyToValue map. Aliases are added to this map and can then be used as if they were normal keys. This created a lot more boilerplate than I'd like, and potentially several O(n) actions if we ever wind up with a ton of aliases registered.I am also uncertain if I also need to create an additional static class and CODEC for this third map.
AI Usage
Outcome
.remap()actually remaps and doesn't break.remove()..alias()does what.remap()used to do. Every.get()and.remove()call now also checks for Aliases existing. 1:1 Key-Value is restored.How Was This Tested
standard game tests passed, Client 1 loaded into SSP + connected to Server SMP, nothing crashed or broke, and I don't see any registry warns or errors
Potential Compatibility Issues
.alias()exists effectively entirely for migration compatibility; the only case of it in the main code base exists to register Rock Breaker conditions as also being Fluid Recipe conditions. If anything this fixes a potential future compatibility issue by fixing the way.remove()broke on.remap()(Should this change go into migration docs somewhere? If so, where?)