niv / neverwinter.nim

CLI tools and nim library used in Neverwinter Nights: Enhanced Edition development
MIT License
133 stars 29 forks source link

add support for binary (0b[0|1...]) and octal (0o[0-7...]) integers #133

Closed tinygiant98 closed 3 weeks ago

tinygiant98 commented 2 months ago

This PR is an answer to a question nobody asked and resolves no known problems. This change allows the use of binary (0[b|B]) and octal (0[o|O]) integers in nwscript. Additionally, it adds a check for the maximum length of the token based on type (8 characters after 0x for hexadecimal, 31 characters after 0b for binary, and 11 characters after 0o for octal).

For the octal integers, I went with the 0o prefix instead of the potentially more traditional numbers-that-begin-with-0 because numbers that begin with 0 are currently valid integers in nwscript and I wanted to ensure backward compatibility.

The whole token-length checking may be superfluous as normal integers are not checked. Binary and Octal code has been kept in their own blocks instead of integrated into hex code blocks to make future modification or removal easier.

Testing

Successfully tested against the following constructs: int h = [+-]0[xX][A-Fa-f0-9...] int b = [+-]0[bB][01...] int o = [+-]0[oO][0-7...]

Example tests: my_function(h) my_function(b + o) int x = 0xff04ab + 0b000000100010001 + -0o03372 my_function(0xff04ab + 0b000000100010001 + -0o03372) if(h) if(0xff04ab) while(b) while(0xff04ab) my_function(++a)

Errors: Compiler will error with UNEXPECTED CHARACTER if a hex number includes characters other than [A-Fa-f0-9], a binary number includes characters other than [01], or an octal number contains characters other than [0-7]. The error TOKEN TOO LONG will be issued if a hex integer has more than 8 digits after 0x, a binary integer has more than 31 characters after 0b, or an octal integer has more than 11 characters after 0o.

Changelog

Added

Licence

mtijanic commented 2 months ago

You should add the bits listed under Testing as actual test cases here: https://github.com/niv/neverwinter.nim/blob/master/tests/scriptcomp/corpus/constants.nss

tinygiant98 commented 2 months ago

You should add the bits listed under Testing as actual test cases here: https://github.com/niv/neverwinter.nim/blob/master/tests/scriptcomp/corpus/constants.nss

Wilco, I completely forgot there was a testing script. Maybe add that to the PR template? Or, I guess, if I have to, I could just be a better programmer/contributor, but that requires work on my part. Fixed in 6cdbd496.

tinygiant98 commented 2 months ago

Note: Hold on to any merges for this ... for some reason, I'm getting errors on any hex number that ends in an alphabetic character. I'll go through that debug on it tonight.

Edit: False alarm, it was a different edit on another branch causing the issue.