search

SHsuperCM / CITResewn

Fabric implementation of mcpatcher's cit
MIT License
146 stars 72 forks source link

fixed: any unknown condition breaks cit completely #369

Open HiWord9 opened 5 months ago

HiWord9 commented 5 months ago

So all i did is i changed ConstantCondition.FALSE to TRUE that will be attached to list of conditions for cit in case a condition is unknown for CITR.

I was making a new feature for my mod, this feature requares a parameter in .properties file. So i implemented it, started testing and my feature works well, but the cit itself breaked. I started looking for cause of its issue and fould out that code adds unreachable condition. I tested on OptiFine and there the cit works fine with my feature. I thought of possibility to use api to register it as my mod's condition, but api is deprecated and i do not want for resource pakcs work only with my mod. I also thought about using commented line in .properties file but its tooooooo strange and not practical and difficult to implement as commented lines are ignored by default and its just not cool as the .properties funcional already gives ability to get values from it.

So, why to make cit unreachable? If there is a misstake in properties file it will be ignored competely. If other mod uses CITR's api (what is most likely not) to add its custom condition, cits with this condition won't work at all without this mod. And as packs are not depend on mod loader, on forge (with OF) it will work, but not with cit resewn. I mean this pull request was not made esspecially for me and my goals, i made it cause i found an issue while reaching my goals and fixed it.

This change should not break any already existed resource pack.

OR may be to add some tag to the beggining of condition line as "IGNORE" or idk mb "::" or any else that will tell CITR that this condition should be ignored and not parsed through?

SHsuperCM commented 3 months ago

So the default of an unknown condition definitely should stay yielding false imo, so it's not really a bug to be fixed and I'd say it's working as intended. However there probably should be a way for other mods to add these flags without breaking the resourcepack. Note however that this is not a condition really but more of a type's effect declaration.

Perhaps a short flag is in order to mark certain lines as "ignore by default"..

HiWord9 commented 3 months ago

So, what i just made is i added a condition to skip a line in properties file if it starts with "%", it means that other mods are able to read their custom values from prop file when citr know that this condition should not be assigned to cit. It also means that if value is something unknown and not marked as citr ignored the citr still will set it as False condition.

SHsuperCM commented 3 months ago

I think this might be the best option but I am not sure about the syntax and where it's been implemented. I'll think about it.