Kenix3 / libultraship

Porting games to the PC
MIT License
100 stars 64 forks source link

move json resource/factory to LUS #496

Closed briaguya-ai closed 1 month ago

briaguya-ai commented 1 month ago

verified working in soh https://github.com/HarbourMasters/Shipwright/pull/4063

briaguya-ai commented 1 month ago

yeah, RawJson isn't the greatest name, it's actually "nlohmann parsed json," i'm ok with just Json but if we want to indicate the nlohmann-ness of it in the name somehow that works for me too

Kenix3 commented 1 month ago

yeah, RawJson isn't the greatest name, it's actually "nlohmann parsed json," i'm ok with just Json but if we want to indicate the nlohmann-ness of it in the name somehow that works for me too

Not sure we need to capture that it's nlohmann personally. Can you think of a reason why we would ever use two different parsing libraries at the same time?

briaguya-ai commented 1 month ago

Can you think of a reason why we would ever use two different parsing libraries at the same time?

i'd expect we wouldn't want to do that, only thing i can think of is if we wanted to move to a different json parsing library it'd be easier to do a deprecation process where we have NLJson and NewParsingLibJson around at the same time, then drop NLJson, but we could probably handle that with feature flags instead. i'll move forward with renaming it to Json

Kenix3 commented 1 month ago

Can you think of a reason why we would ever use two different parsing libraries at the same time?

i'd expect we wouldn't want to do that, only thing i can think of is if we wanted to move to a different json parsing library it'd be easier to do a deprecation process where we have NLJson and NewParsingLibJson around at the same time, then drop NLJson, but we could probably handle that with feature flags instead. i'll move forward with renaming it to Json

If the argument is for supporting new and old json libraries in a transition period, my suggest would be to call it Json and rename it to JsonLegacy when we need that.