lep / pjass

The famous jass syntax checker.
BSD 2-Clause "Simplified" License
25 stars 6 forks source link

Add test for too long variable names #5

Closed Luashine closed 11 months ago

Luashine commented 11 months ago

https://github.com/Luashine/wc3-test-maps/tree/master/variable-length-crash

In Jass, variable length of exactly 3959 or above will crash old versions of the game when loading "war3map.j". This is true for locals and globals.

Edit:

Appears to have been fixed in Reforged (1.33.0.19203). I didn't check which version.

Both war3map.j and the map are included here.

Currently: no warning, no error.

lep commented 11 months ago

I'm a bit uneasy about this one. In my mind i only ever really wanted to support the latest patch. This is obviously false when you look at all the supported flags but my main focus was the latest patch anyways. Now this, as you report, seems to be fixed in the latest patch and while very interesting, hitting this limit is very unlikely. I'm more willing to add a flag for #4 because using the modulo operators seems way more likely than hitting this limit. I'm happy about any comment but for now i have no idea what i want to do about this.

Luashine commented 11 months ago

I understand but the community is going to become more split in the future. The current release state is not much better than early 1.32.x, Mindworx et al working on W3CE based on 1.29 (though they could be asked for bug fixes unlike Blizzard), take LAN communities and the yet more fractured RU community into account - all of who are keen to be using 3rd party tools (themselves). Idk what the Chinese are doing, but judging by the lack of documentation and file format work (I did end up translating search results from there) - they must be using our tools too.

This is a really strange edge case, even vJass would be unlikely to produce such a monster (it does unfold its structures into global functions and variables). If you don't want to work on this, please leave the test case in a new folder "undecided", I really hate looking for scattered information on Hive and other places if you couldn't tell :)

While at it: type name length didn't matter when at limit and I guess value length too (unless strings etc?)

lep commented 11 months ago

Another idea that came to my mind is to simply have another git branch for old(er) patches. pjass isn't high-volume so i could reasonably support both. This could include a whole bunch of stuff like modulo aswell.

Luashine commented 11 months ago

I think this is too much hassle. And consider having two versions out in the wild, this would add more confusion than it'd help.

Luashine commented 11 months ago

I thought to comment here instead in the PR. MindWorX was honest when he said "they blew most of the limits in Reforged"

local max OK len: 3958 
global max OK len: 3958 
function max OK len: 3958

comment length:
    v1.27: the entire line must be <=3958 in length (also counting // and Carriage-Return as characters) or it will crash
    1.32.10: all OK

extends type (v1.27):
    3958 LF ok
    3958 CRLF ok
    3959 LF crash
    3960 LF crash

    1.32.10: all OK

funcname
    3958 CRLF OK
    3958 LF OK
    3959 LF crash
    3959 CRLF crash

func argument name length (v1.27)
    3958 LF OK
    3959 LF crash

    Reforged: OK

Return string inline length (v1.27)
    750 OK
    1000 OK
    1020 OK
    1023 OK
    1024 other
    1025 other
    1200 other

    1500 other
    3000 other crash
    3950 LF other crash
    3958 LF crash
    3959 LF crash

    v1.32.10: 3959 LF length: OK!

Local string length inline (v1.27)
    1023 OK
    1024 Crash

    v1.32.10: All OK

Long type name and long var name:
    both var type and local variable name can be at their respective max length

I don't know if the error codes/locations are specific to my patch, but the internet is quiet about them. I assume this is a good thing in that nobody was crazy enough to encounter these errors :)

Since I was playing with types anyway, it's impossible to extend types other than handle or the game's verifier will discard the script.

Extend types (v1.27):
    code NO
    handle YES
    boolean NO
    real NO
    integer NO
    string NO
    "nothing" NO
    "void" NO
    "function" NO

Just in case here's a script for very quick iterations. This is blazing fast with old versions while Reforged takes age to load. Alt+F4 the game when done:

"C:\Programs\Blizzard\mpqeditor_en\x64\MPQEditor.exe" add jsyn.w3x war3map.j
cd "to/war3/gamefolder/
War3.exe -loadfile "X:\v1.27\Maps\jassSyntax\jsyn.w3x" -windowed

I will upload w3x maps (v1.27) and war3map.j files along with Windows' crash messages to my repo in a few minutes: https://github.com/Luashine/wc3-test-maps/tree/master/variable-length-crash