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.

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

Make editor work with current version of the game #29

Closed
wants to merge 6 commits into from

Conversation

girtonman
Copy link

After doing some troubleshooting, it looks like the main issue that was causing it to crash was that the seed for the save was too big for a js float, so it was being rounded.
For example:

  "seed": 1461819000922140955,

was being rounded to

  "seed": 1461819000922141000,

This would cause the game to crash when the map gen for the rounded seed would conflict with the map position info in the save file.

Adding lossless-json solves this issue pretty easily. I did some very simple testing and the tool seems to be working again with this fix.

@girtonman
Copy link
Author

Closes the following issues:
#25 #27 #28

@zuelong
Copy link
Owner

zuelong commented Jun 3, 2019

Wow! Thanks so much for digging and looking into this! I'll be sure to get this deployed and maybe add some CI so I can stop doing manual deploys 😅

Copy link
Owner

@zuelong zuelong left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Just a few suggestions before I can merge this in, but great work!

I'll get it tested and merged after these are corrected

src/components/Advanced.js Outdated Show resolved Hide resolved
src/components/Advanced.js Outdated Show resolved Hide resolved
src/components/FileUpload.js Outdated Show resolved Hide resolved
@girtonman
Copy link
Author

girtonman commented Jun 3, 2019

I've also been messing around with it and found a couple minor changes that need to be made to handle the new LosslessNumber type.

Throughout a day of testing and using it, I'm feeling even more confident in this fix.

I'm in the process of adding all of the new cards/relics to CardsJSON and RelicsJSON. Would you like me to shoe-horn them into this PR or make a new one for that? There are about 90 cards that need to be added. 😮 I haven't figured out how many relics yet though, but that should hopefully be a shorter list.

@girtonman
Copy link
Author

Resolved all feedback and also made a small fix to handle the fact that the value of upgrades can be a number of a LosslessValue depending on its source (file upload or card selector),

@girtonman
Copy link
Author

Also moved the changes I was talking about for CardsJSON and RelicsJSON to another branch that I can make a PR for once this one is wrapper up.

girtonman and others added 4 commits June 13, 2019 01:51
Since `upgrades` can be a `LosslessValue` or a number, the ternary condition needs to handle both.
@girtonman girtonman closed this by deleting the head repository Nov 14, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants