Refactor LD to use StringName item IDs instead of ints #301
Conversation
This file contains 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
Finally figured out how and why data is being written from functions which seemingly return nothing: they are being given arrays and dictionaries, which are always stored and passed by reference--then saving *inner dictionaries" by reference, and writing the original passed arrays through the reference. With this new knowledge, I can comment about what's going on in the code so future readers aren't confused (hopefully), and also finally document the format of the item properties dictionaries!
All these fns use the same sequence of operations, and if they can rely on by-reference semantics, so can I ;)
...and also properties' "increment" property, since I apparently missed documenting its default value.
...now that I finally understand what it does.
To facilitate using human-readable IDs in JSON-serialized level files. Not too hard a change either. Just following the errors fixed everything up--no huge logic changes or anything. Didn't even have to touch most uses of these dictionaries! Surprisingly, the item list is even ordered the same. On that note: I haven't actually changed any item IDs yet. Lucky they're already strings.) (Accidentally got into fixing a few crashes in exit-playtest mode. Still crashes, but further along.) (Still not worth getting it totally working though.)
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
Description of changes
Exactly what it says on the tin. Also, comment and cleanup (I had left some functions literally marked "does nothing" because I couldn't tell what they output--even though I knew commenting them out broke the code--so now that I've figured it out, I've gone back and clarified).
Issue(s)
Meant to facilitate using human-readable item IDs when we switch to JSON level serialization.
Future work
items.xml.tresstill uses ints for its IDs. The ints are stored as strings, so it still works, but it's far from human-readable yet.