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

units::volume and units::mass readers for mandatory and optional #36354

Merged
merged 1 commit into from
Dec 24, 2019

Conversation

KorGgenT
Copy link
Member

Summary

SUMMARY: Infrastructure "implement volume_reader and mass_reader for read and write"

Purpose of change

The purpose of this is to make it simple to read and write various units using optional and mandatory from generic_factory, and also able to serialize easily. The usage for this is pretty straightforward: just add one of the readers as a parameter to optional or mandatory. this will avoid seg faults and having to re implement reading json. The reader does not support legacy units, and we should eventually get away from using legacy 1 = 250 anyway.

Describe alternatives you've considered

writing some kind of helper funciton i suppose? that's basically what this is though

Testing

Ran the game in vanilla, found some errors. talked to anothersimulacrum about it and he PRed several changes already. This needs to have tests run to make sure that in-repo mods are all prepared for it, as I changed one of the loading functions to use this new reader as a proof of concept.
It would be cool to use this for items and monsters, but those have their own specialized classes rather than using generic_factory.

@KorGgenT KorGgenT added [C++] Changes (can be) made in C++. Previously named `Code` 0.E Feature Freeze labels Dec 22, 2019
@KorGgenT
Copy link
Member Author

ran cata tests with every single mod individually and didn't see any errors. i don't know what's up with the template error but i've got some ideas. in the meantime i'm undrafting this as it works perfectly

@KorGgenT KorGgenT marked this pull request as ready for review December 23, 2019 14:45
src/units.cpp Outdated Show resolved Hide resolved
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
[C++] Changes (can be) made in C++. Previously named `Code`
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants