-
Notifications
You must be signed in to change notification settings - Fork 30
Fix Grown Tardis Item crashing the game if Valkyrien Skies is installed. #579
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
Open
Acuadragon100
wants to merge
4
commits into
WhoCraft:dev/1.20.1
Choose a base branch
from
Acuadragon100:dev/1.20.1-fix-create-item-vs
base: dev/1.20.1
Could not load branches
Branch not found: {{ refName }}
Loading
Could not load tags
Nothing to show
Loading
Are you sure you want to change the base?
Some commits from the old base branch may be removed from the timeline,
and old review comments may become outdated.
Open
Changes from all commits
Commits
Show all changes
4 commits
Select commit
Hold shift + click to select a range
1b13587
Fix crash when using the new Grown TARDIS item with Valkyrien Skies.
Acuadragon100 1f8982c
Changelog
Acuadragon100 8742a65
Serialize the setup state.
Acuadragon100 7cd9d46
Merge branch 'dev/1.20.1' into dev/1.20.1-fix-create-item-vs
Acuadragon100 File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
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
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
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
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
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
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.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It has been a while since I've been in here. Interesting use of the record here. Does this need to be serialized in any way?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I just felt that a record was a nice and compact way to keep all parameters in one place.
I did consider serialising it as well, but thought it was unnecessary considering it will only exist for a single game tick, especially since it's only used when Valkyrien Skies is installed. Also, the Grown TARDIS item is a bit of a creative-mode quality of life item, it even has infinite uses in survival mode (is this a bug?).
The main potential concern I could see is that the shell won't re-trigger the setup on it's own when right-clicked (unlike the root shell), so if the server shuts down after using the grown TARDIS item before the next game tick (very unlikely) it will create an unbreakable (even in creative mode, although it can removed with /setblock) and unusable TARDIS shell without an interior.
Serialising it would solve this, but another solution could be to just have the shell delete itself if it doesn't have a valid TARDIS_ID (which it probably should do anyway in my opinion). Let me know which approach you prefer, I'd probably serialise it if you want to future proof things for a potential survival-mode-friendly way to create a TARDIS directly without the root shell.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actually, on second thought I decided it would probably be best to serialise it to future-proof the mod.
This also makes it possible to create a TARDIS manually by just placing the shell block with a command, which could be useful.