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.

Sign up for GitHub

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

Jump to bottom

Fix Relational Variables #3961

Closed
wants to merge 18 commits into from
Closed

Fix Relational Variables #3961

wants to merge 18 commits into from

Conversation

APickledWalrus
Copy link
Member

@APickledWalrus APickledWalrus commented May 8, 2021 •
edited
Loading

Description

Reopening of #3582 (note: I have not touched anything, just reopened this for the reason of continued discussion)

This PR fixes the NotSerializableException when PersistentDataHolder is a registered ClassInfo. Relational Variables are enabled again too. This does not fix the actual issue with Yggdrasil, so it is more of a workaround. If anyone has any ideas for solutions, I am open to hearing them.

edit: thanks to tpgames for helping me out ;D

Target Minecraft Versions: 1.14+
Requirements: None
Related Issues: #3185

@TPGamesNL TPGamesNL added 2.6 bug An issue that needs to be fixed. Alternatively, a PR fixing an issue. labels May 9, 2021
@APickledWalrus APickledWalrus marked this pull request as draft May 9, 2021 17:41
@APickledWalrus
Solution and code improvements
02105de 
- Better fix for item serialization (reverts old one)
- Code improvements (fixes stuff like 'set {data} of player's tool to "data"')
@APickledWalrus APickledWalrus marked this pull request as ready for review May 9, 2021 19:53
@TPGamesNL TPGamesNL mentioned this pull request Aug 12, 2021
Merged
@APickledWalrus APickledWalrus added feature Pull request adding a new feature. and removed 2.6 labels Aug 12, 2021
@APickledWalrus APickledWalrus added this to the 2.7 milestone Aug 12, 2021
@APickledWalrus
Copy link
Member Author

A few things to look at:

  • User friendliness of syntax
  • Performance
  • Missing syntax (e.g. does loop-index work) (I don't think it currently does, but it may not even be feasible)

@APickledWalrus APickledWalrus removed this from the 2.7 milestone Jun 6, 2022
@APickledWalrus APickledWalrus added the up for debate When the decision is yet to be debated on the issue in question label Jun 6, 2022
@TheLimeGlass TheLimeGlass modified the milestone: 2.7 Jul 1, 2022
@TheLimeGlass TheLimeGlass added the 2.7 Targeting a 2.7.X version release label Jul 1, 2022
@Moderocky Moderocky requested a review from Pikachu920 October 10, 2022 15:49
@APickledWalrus APickledWalrus mentioned this pull request Oct 10, 2022
Merged
@TheLimeGlass
Copy link
Collaborator

Closing, see #5158

@APickledWalrus APickledWalrus deleted the fixes/persistent-data branch September 6, 2023 18:48
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@TPGamesNL TPGamesNL TPGamesNL left review comments

@FranKusmiruk FranKusmiruk FranKusmiruk approved these changes

@Pikachu920 Pikachu920 Awaiting requested review from Pikachu920

Assignees
No one assigned
Labels
2.7 Targeting a 2.7.X version release bug An issue that needs to be fixed. Alternatively, a PR fixing an issue. feature Pull request adding a new feature. up for debate When the decision is yet to be debated on the issue in question
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
5 participants
@APickledWalrus @TheLimeGlass @Pikachu920 @TPGamesNL @FranKusmiruk