[discussion needed] wurstscript should check validity of collection of asiiInts passed to objediting?

[Cokemonkey11]

trafficstars

Frotty and I had a hell of a time debugging really strange techtree and object data behavior that arised because I set some units with strings like 'f000', 'f001', 'f002' etc, and some abilities with 'F000', 'F001', 'F002' etc.

• wc3 only treats a capitalised first letter for changing heroes to units, any other use can be problematic
• in general, mixing 'f000' and 'F000' object IDs is allowed, but for techtree requirements and other situations seems to be undefined behavior (widgetizer for example will really muck this up)
• There seems to be some safe cases, for example where techtree requirements are not involved but a unit 'c000' has ability 'C000' - that seems to work, but in general this is risky business

I propose we augment the wurstscript objediting subsystem to do some compiletime checks on all the values passed to objediting as asciiInts and raise a warning for any time when a unit, hero, or item mixes 'zAAA' with 'ZAAA'.

@lep might be interested to join this discussion?

[peq]

I think those checks could be added in the standard library.

[Frotty]

transfer this ticket to stdlib repo @Cokemonkey11 ?

[Cokemonkey11]

@Frotty I don't know much about compiler objediting subsystem or the stdlib, but I do think, given that this involves code that modifies the mapfile, and the compiler toolchain's job is to ensure the mapfile is valid, this feels like something that belongs in the compiler.

Because otherwise it means anyone not using the STL to do something objediting related is going to have a chance to generate a map that does something totally bonkers.

So overall I was waiting for your opinion too. Do you agree with peq that this isn't something the compiler wants?

[Frotty]

It depends. If you want Wurst to generally check for overlapping ids, then it would require a compiler feature. If you just want to make sure 2 overlapping ids aren't passed to new ObjectDefinition then this could be done in the stdlib. The stdlib approach would probably give you better/direct error description and trace, but can't detect e.g. overlaps within the map.

[Cokemonkey11]

I understand the implications yeah. But what do you think?

Ultimately I think the right thing is for the compiler to own this, but in practice an STL fix would probably cover ~100% of use cases.

[peq]

If there are some strict checks that mus be fulfilled, we can add them here: https://github.com/wurstscript/WurstScript/blob/master/de.peeeq.wurstscript/src/main/java/de/peeeq/wurstio/intermediateLang/interpreter/CompiletimeNatives.java#L34

But if these are just things that might go wrong when used in certain scenarios, I would not complicate the logic in the compiler too much here and rather add checks to the standard library. I think the standard library is easier to understand for users, because they can just jump to the source of the error, read what the conditions are, and adapt them if they think they know better.

[Cokemonkey11]

I think you're right, I've overlooked the fact that this isn't guaranteed to go wrong.

To be clear: when it does go wrong, it's very hard to debug as it seems to be some undefined behavior:

• SLKs after widgetizer still appear okay.
• In-game, weird stuff happens where unrelated object-id reference handling breaks - for example, a unit might lose its techtree requirements, or have a new techtree requirement that just says "Default string"
• The especially unlucky author (me) might mask weird bugs like this by making objediting changes that make the map work "okay" again - actually just hiding the real problem.