NFORenum v3.4.6 released (NFO renumberer and linter)

[DaleStan]

If anyone has sprites with UTF-8 encoded strings (The more multi-byte sequences, the better), would you please post them here, or PM them to me? (preferably hex, not quoted; I don't want to risk transmission lossage.)
I'd like to test NFORenum's obedience to the don't-break-UTF-8 rule and the QUOTEUTF8 setting.

No rush, though; I still have plenty of other things to do.

(minor update to the style doc.)

[George]

I'll try again. It's easy to determine that you are definitively NOT in a counted jump, but you can not reliably determine that you are in a counted jump until you see the corresponding action 10:

Code: Select all

0*0 07 ... F0
0*0 02 02 10 83 F2 00 FF 04
00 80 00 00 //...
01 80 01 01 //...
02 80 02 02 //...
03 80 03 03 //...
04 80 //...

Now, is that action 7 a jump or a goto? And how do you know? Remember, you can't go on to the action 2, or any following sprite, until you've decided what to do with the 7.

Hm, you are right. Previously 00-df were jumps and e0-ff were gotos, but now that does not work. that means the parser should make at least two runs to work with it. What do you think about it ?

I think it is a BAD solution. Props would visually mess with string numbers.

That's the system I use, and I haven't had troubles.

I've seen only your nfo files for old versions of a plane sets, and the only mullti-line strings are

Code: Select all

 770 * 46	 00 03 12 01 0D 00 AE 47 02 20 03 19 04 2F 06 07 07 1E 08 FF 09 02 0A 01 0B 5A 0C 46 0D 07 0E D2
	 0F C2 01 11 64 12 07 13 FF FF FF 1F 14 20
 807 * 46	 00 03 12 01 0E 00 00 5A 02 1E 03 19 04 FF 06 07 07 1E 08 FF 09 02 0A 01 0B 5F 0C 47 0D 07 0E D7
	 0F E0 01 11 64 12 07 13 FF FF FF 1F 14 20

so there is no place to mistake. I use many mullti-line strings and for me this would be unreadable

Code: Select all

 6394 * 48	 02 09 16 04 
D6 11 00 00 
17 00 00 80 	 00 00 00 08 08 20
16 00 00 80 	 08 00 05 08 08 20
14 00 00 80 	 00 08 05 08 08 20
15 00 00 80 	 08 08 0A 08 08 20
 6395 * 48	 02 09 17 04 
D6 11 00 00 
17 00 00 80 00 00 00 08 08 20
16 00 00 80 08 00 05 08 08 20
15 00 00 80 00 08 05 08 08 20
14 00 00 80 08 08 0A 08 08 20
 6396 * 71	 02 09 A3 80 01 00 20 00 00 01 00 02 00 03 00 04 00 05 00 06 00 07 00 08 00 09 00 0A 00 0B 00 0C
00 0D 00 0E 00 0F 00 10 00 11 00 12 00 13 00 14 00 15 00 16 00 17 00 00 00 03 00 06 00 09 00 0C
00 0F 00 12 00 15 00

I suggest t add one more param - number of leading spaces for the second and later strings of multyline string.

I also want lines-breaks added by the beautifier and lines breaks added to obey the max-line-length rules to be clearly distinguishable; should they be two separate options, or is it OK to just add an additional three spaces when the line-lengther breaks the lines?

I'd suggest separate options

Think of sentences, paragraphs, chapters....

I do not think it is good [analogy] here

English has words, which are followed by a space; phrases, which are followed by a comma; sentences, which are followed by a a period; and paragraphs, which are followed by a carriage return.
NFO has bytes, which are followed by a space; "spritelets", which are followed by a carriage return; sprites, which are followed by a carriage return and a new sprite header; and sprite blocks, which are followed by a blank line. So it doesn't match quite perfectly. What analogy does?

The main different between a book and a code - you never scroll the book fast. The LB is seen good on the static peace of paper, but the ---------------------------------------- is seen much better when you do a scroll. Btw, in books, that are not designed to be read from first page to the last, usually have a huge visible on fast page searching headers with lines above them to make it faster to find the text.
But may be it is rather personal.

Nobody put his NFO with GRF file into zip(rar) file.

Hi! I'm nobody! Pleased to meet you!

The only zip with nfo I have is some versions of plane set. Other zip (your industries I have) do not have them. And I suppose that is not a good solution, because most user would get useless files in their grf folder, because people usually extract all the archive at once. Storing comments inside the grf would look like decompiling - you need to use a tool to get a readable form.

The main idea of my suggestin - to store development information (comments) inside the grf file. Where else, than action C, can you store it?

If you want action Cs, write action Cs. If you want comments, write comments. NFORenum doesn't really care.

if I write action c, it would get a 1 * 1 header, that would make it harder to see it

By combining the comments you have made the code less legible, that's what I'm saying.

Looks like you missed the idea. it should be stored in the action C before action (2), but should be placed back into it for NFO editing.

A few minor problems there:
1) The comments can't be added to the action 2 until the pretty-printer works.
2) New sprites (such as the action Cs) can be generated, but cannot safely be inserted except by a comment command, after a real or include, or at the end of the NFO.[0]
3) Even if the new sprites could be safely generated, they would appear AFTER the action 2, not before it, making it impossible to break the C and re-attach it to the action 2

would it be possible to solve the problem if the tool would use several levels(runs?) of a parser?

[DaleStan]

that means the parser should make at least two runs to work with it.

Multipass linter

I've seen only your nfo files for old versions of a plane sets, and the only mullti-line strings are...

That's because most of my NFOs get massaged through grfcodec multiple times, and I only pretty-print it when I first write it or actually need to read it. Here are a couple random sprites OilPower, in my standard format:

Code: Select all

   16 * 120	 00 0A 0A 01 00
08 01
09 1B
0A 01 52 00 00 00
   00 00 FE 00 00
   01 00 FE 00 00
   02 00 FE 01 00
   03 00 FE 02 00
   00 01 FE 00 00
   01 01 FE 00 00
   02 01 FE 01 00
   03 01 FE 02 00
   00 02 FE 00 00
   01 02 FE 00 00
   02 02 FE 03 00
   03 02 FE 04 00
   00 03 FE 00 00
   01 03 FE 00 00
   02 03 FE 04 00
   03 03 FE 04 00
   00 80
11 03 FF FF FF
16 0B 05 FF
17 08
18 02
19 BD
1A 00 08 00 00
1F 00 DC
...
  104 * 38	 02 09 0F 03 00 00 00 80
22 00 00 80 00 00 00 0F 0F 0B
2F 00 00 80 0F 00 00 00 0F 0D
30 00 00 80 00 0F 00 0F 00 0D

(note that this is not an option in NFORenum; a minimum of one space is added at the beginning of every line.)

The only zip with nfo I have is some versions of plane set. Other zip (your industries I have) do not have them.

Looks like you have a point there. Well, I never actually put them both in the same zip, but I did provide NFO and PCX for the planes I coded, and I intended to do so for the industries.

would it be possible to solve the problem if the tool would use several levels(runs?) of a parser?

Again, a multi-pass linter is already on the todo. It's probably necessary but not sufficient.

[DaleStan]

v2.6.0 to v2.6.1
- (bugfix) Really fixed the broken param checking for 60+x vars.

Thanks a lot, folks. You're REALLY making me feel like I'm writing a useful program.</sarcasm> Maybe the 2.6.1 I released was fixed, but the archived source I have for that version says otherwise. As does the source for 2.6.2, 2.6.4, 2.8.0, and 2.8.4, among others.

I would *really* have appreciated it if someone had told me "No, you rebroke that instead" before now.

Apparently I have not made this properly clear:
If you report a bug, and then I claim to fix it, please check that it actually *is* fixed. And if it isn't fixed, yell at me.

(Most of the anger here is directed at the person who reported that bug, but didn't complain when I unfixed it. The remainder applies to everyone.)

[OzTrans]

v2.6.0 to v2.6.1
- (bugfix) Really fixed the broken param checking for 60+x vars.

Are you talking to me ? If so, I haven't noticed any adverse effects, but then I still use v2.6.2 (plus a few updated .dat files) and will do so until I get stuck, then I shall upgrade and should I have still problems, only then shall you hear from me.

BTW, this is an extremely useful tool and it does get used a lot - thanks for all the work you put in.

[DaleStan]

I should have tested it, and I usually do test my bugfixes. Either 1) I failed to test it properly, or 2) I unknowingly reverted the fix after I tested it. I don't know which happened here, but given the other things that were happening at the same time, I'm guessing that one of my UTF-8 support reverts also reverted the the fixed param checking.
I realize that people do change their nfo files on occasion, and that this may cause them to no longer trigger bugs, but I find it extremely disheartening to discover that a bug I reported properly fixed two months ago was only fixed for the week between the datafile update and the release.

Hopefully, this problem will never come up again, but if it does, now you know: The appropriate procedure is to re-report the bug.

In other news, the NFO specs have changed, warranting 2.8.6, and I think the beautifier is behaving well enough for general release, warranting a pre of 3.0.0 (which can be found here.) EDIT: For 3.0.0p1, what I'm most interested in is the persistence of comments and blank lines, and the obedience to QUOTEHIGHASCII and MAXLEN. QUOTEUTF8 and HEXGRFID will both fall back to QUOTEHIGHASCII for now.

v2.8.5 to v2.8.6/v3.0.0 pre 1
- Made comment commands case insensitive (except for variable names).
- (v3.0.0 pre 1 only) Made undef @@BEAUTIFY options and settings complain instead of set default.
- More a73 updates:
- - Callback 3B.
- - Industry production callback's <again> may have any value.
- - Industry variable 61.
- - TextIDs F8FB..F8FF.
- (3.0.0 pre 1 only) Enabled the beautifier.
- (2.8.6 only) diked out @@BEAUTIFY, &c.

v2.8.4 to v2.8.5
- (bugfix) -? message now says @@REALSPRITES.
- (bugfix) Checked to ensure that pseudo continuation lines were being attached to pseudosprites, rather than turning into independent sprites.
- Added @@BEAUTIFY/--beautify/-b. (No beautifier to control, though.)
- Added '=' as an acceptable delimiter for comment commands.
- Changed @@DIFF to imply @@LINT NONE.

[OzTrans]

v2.8.6 and v3.0.0

I have test driven the 2 with the features used in my sets and have not come across any problems. Good job.

[DaleStan]

Good to hear, OzTrans.

Actions 0 and F are going to be rather more difficult than the others, but with pre2, everything else should be functioning as advertised. It's in the same place as pre1. 2.8.7 (just the last two items below) will be up shortly.

If you've got cookies from 3.0.0p1, you'll have to regenerate them. I'm still open to the concept of adding more settings, but I'll try to keep old cookies valid from now on.

v3.0.0 pre 1 to v3.0.0 pre 2
- Added pretty-printing for all actions except 0 and F.
- Added pretty-printing for recolor maps and binary imports.
- All strings now obey QUOTEUTF8.
- All GRFIDs now obey HEXGRFID.
- Changed beautifier's cookie format.
- Changed LEADINGSPACE to allow three independent settings.
- Changed line breaks to start 15 characters before max.
- (bugfix) GETCOOKIE returns a valid comment command.
- (bugfix) Corrected NOPRESERVE's meaning when following @@BEAUTIFY
- (bugfix) UTF-8 encoded Thorn is C3 9E, not C3 93.
- (bugfix) Action 7/9 test 06 is a GRFid test.

[Csaboka]

I've just finished finding a bug for George, and it gave me an idea for a new feature.

The problem was that George accidentally typed "1E 2E" instead of the intended "1E 1E", and this broke his variational action 2. It took me a while to spot the typo since I expected the two values to be the same.

My suggestion is: add a check for overlapping ranges in variational action 2s. A warning for this would prevent most of the mistakes similar to this one. At least, there should be a "branch in variational action 2 can never be reached" warning for people who intentionally add overlapping ranges. (A branch is unreachable if the preceding branches cover all of its range.)

(I haven't followed the evolution of NFORenum closely, so feel free to apply a clue-by-four if it's already suggested/done... A quick search in sanity.txt didn't show up anything like this, though.)

[DaleStan]

I've just finished finding a bug for George, and it gave me an idea for a new feature.

add a check for overlapping ranges in variational action 2s. A warning for this would prevent most of the mistakes similar to this one. At least, there should be a "branch in variational action 2 can never be reached"

A quick search in sanity.txt

I'm afraid you picked the wrong file to check. It's in todo.txt. The main reason it hasn't happened is that I haven't yet come up with any particularly good way of dealing with non-adjacent ranges, especially in 85/86/89/8A varactions.

v3.0.0 pre 2 to v3.0.0
- Added pretty-printing for action 0 and F.
- Several beautifier bugfixes/changes:
- - Obey LINELENGTH.
- - Add missing newline between <message> and <data> in action B.
- - Whitespace between code and comment from the previous line(s) in a sprite no longer added to the current line.
- - Do not break if the remaining bytes are guaranteed to fit.
- Action F updates:
- - Uses the action 4/8/B string parsing routines.
- - New language byte format applies to action F too.
- Added --lock, to prevent NFO files from overriding the commandline.
- (bugfix) "Unexpected EOF: Unused town name IDs" message goes to the console, not the NFO.
- (bugfix) Make -k not cause crashes.
- Add check for presence of <description> in action 8.
- (bugfix) Action Bs with message-id FF now check <data> ...

[Csaboka]

I'm afraid you picked the wrong file to check. It's in todo.txt.

Oops. That should have been the second file to check...

The main reason it hasn't happened is that I haven't yet come up with any particularly good way of dealing with non-adjacent ranges, especially in 85/86/89/8A varactions.

Finding overlaps would be easy: just check all previous ranges, and if any of them has an overlap, show the warning. Detecting unreachable branches would be a tougher task, of course. My idea is making a linked list of the continuous parts of the range, ordered increasingly. You'd have to check for already existing adjacent ranges and merging two of them in some cases, but I think it should work. (The only problem would be the many malloc() and free() calls, I think, but maybe a 255-element array would do as well, since the maximum number of branches is 255)

[DaleStan]

Actually, I came up with a solution not too long after I posted that. What I'm currently doing is to check if min or max is in any previously defined range, and if so, complain and adjust appropriately (increase min/reduce max) to move it out of that range. When this is done, if min>max, then it's unreachable. This doesn't complain about "... <ID> 10 1F <ID> 00 2F ...", though, but I'm not sure that's really a problem.

And I'm using C++, so I don't deal much with malloc/free. Mostly, I use whatever format(s) happen to underly std::vector and std::string.

Also, I got curious about the various compiler optimizations, and I'll report the results for others who are curious:
I currently optimize for size, and it takes about 8-9 processor-seconds to churn through my 169 file, 6.77MB test directory.
Optimizing for speed increased the size from 96.5KB to 109.5 KB, and reduced the time to 6-7 processor-seconds.

In other news:

v3.0.0 to v3.0.1
- Added variational range accessibility checks.
- 2.0.1 alpha 74 updates:
- - Widened industry variable 61 to a word.
- - General variables prop 10.
- Added a missing \n to --help message.

[DaleStan]

So, quick quiz: What's 1<<32?
If you answered 0, as I did, you'd be wrong. At least for shl edx,cl;
Meaning that NFORenum became rather confused as to the actual range for type 89/8A varactions.

v3.0.1 to v3.0.2
- (bugfix) Check proper range when checking default on type 89/8A varactions.
- (bugfix) Action 0 feat 8 prop 10 is 384 bytes.
- Added pretty-printing for action 0 feature 8 prop 10.

[DaleStan]

Csaboka changed things, so I get to change things too.

New 2v.dat and 5.dat, which can be found in the data-3.0.2.1/ directory.

[DaleStan]

2v.dat has been updated again, and I put a semi-decent index.html up too.

[DaleStan]

Lots of new checks and changes here. No time to be verbose; I should be asleep.

v3.0.2 to v3.1.0
- Varaction range checking now checks ranges for agreement with <var-adjust>.
- Randactions are checked for using valid bits and triggers.
- Added check for both high bits set in varaction <shift>s.
- Reverted "Do not break if the remaining bytes are guaranteed to fit" in favor of an explicit "no line break here" when line breaks do not belong.
- (bugfix) Action F checker occasionally requested one too many random bits.
- (hotfix) Pulled industry var 5F.
- (hotfix) Added industry var 62.
- (hotfix) Added type 0C action 5s.
- (bugfix) Removed extraneous "m/" from message when a datafile read failed.
- Fully functional UTF-8 parser:
- - Encoded Thorn is C3 9E. (again, grr...)
- - Encoded control characters are now checked.
- - 7B..7E in UTF-8 strings are quoted.

[DaleStan]

Grr... At least one of my test suite or test system was broken. Have to be, for me to miss the fact that I was looking for UTF-8 sequences in Latin-whatever_it_is strings, and not in UTF-8 strings.

Now that TTDPatch has gone beta, NFORenum can also relatively simply insert checks for beta versions.

v3.1.0 to v3.1.1
- (bugfix) Check for UTF-8 in UTF-8 strings, not Latin-TTD strings.
- Added 2.0.1a<num>, 25b<num>, and 2.5b<num> formats to VERSIONCHECK.

[OzTrans]

NFORenum v3.1.1 ...

... a few things that popped up while using v3.1.1 ...

Variable space with offset 05h ...

Code: Select all

//!!Warning (143): Offset 6846: Using unspecified control character 05.
 ... 67 65 74 0D 01 05 27 4E 6F 72 74 68 6C ...
                 ^^^^^

... that is correct IMO.

Why not just an 'and-mask' ...

Code: Select all

//!!Warning (172): Offset 5: A shift-and var-adjust would suffice here.
 5993 * 16     02 00 D1 81 41 80 FF 00 02 01 03 FF 01 01 01 FF

to code something like this ...

Code: Select all

 5993 * 16     02 00 D1 81 41 00 01 01 03 FF 01 01 01 FF

What is actually wrong with the following ? The 16 selections should be randomized, when receiving new cargo after the vehicle was empty ...

Code: Select all

//!!Error (129): Offset 5: Only 8 random bits are available.
 1520 * 39     02 00 ED 80 01 05 10 00 00 01 00 02 00 03 00 04 00 05 00 06 00 07 00 08 00 09 00 0A 00 0B 00 0C
                 00 0D 00 0E 00 0F 00

... and it goes with these 4 random selections which are determined at the time of purchase only, i.e. 4 static and 16 dynamic ...

Code: Select all

//!!Warning (174): Using an undefined trigger.
 1555 * 15	 02 00 AE 80 E0 01 04 ED 00 EE 00 ED 00 EF 00

and thanks for the other newly issued warnings, which will make me to change some of the code.

[DaleStan]

Code: Select all

//!!Warning (143): Offset 6846: Using unspecified control character 05.
 ... 67 65 74 0D 01 05 27 4E 6F 72 74 68 6C ...
                 ^^^^^

Fixed. (I put the instruction to skip that 05 in the wrong place.)

Why not just an 'and-mask' ...

Code: Select all

//!!Warning (172): Offset 5: A shift-and var-adjust would suffice here.
 5993 * 16     02 00 D1 81 41 80 FF 00 02 01 03 FF 01 01 01 FF

to code something like this ...

Code: Select all

 5993 * 16     02 00 D1 81 41 00 01 01 03 FF 01 01 01 FF

You seem to be suggesting that the error message be changed to "An and-mask would suffice here"?
I mention both because I use this message for both unnecessary add-mods and unnecessary add-divides. (eg: 40 FF 00 08 -> 03 1F) In the latter case, both shift-num and and-mask need to be adjusted.

Code: Select all

//!!Error (129): Offset 5: Only 8 random bits are available.
 1520 * 39     02 00 ED 80 01 05 10 ...
                              ^^ ^^

You're starting at bit 5 and using 4 bits. Unless Patchman numbers bits starting at 1 here (which would be rather out of character, IMO), that requires 9 bits, but only eight are available. I am pointing at the wrong byte, though; that should say "Offset 6:"

Code: Select all

//!!Warning (174): Using an undefined trigger.
 1555 * 15	 02 00 AE 80 E0 01 04 ED 00 EE 00 ED 00 EF 00
                          ^^

You seem to be trying to use triggers 4, 5 and 6 here. Of those, only trigger 4 is defined.

[DaleStan]

And here's the release.

v3.1.1 to v3.1.2
- Enable pretty-printing of overlength action 0s.
- Tightened resultant range from shift-and-add-mod varadjusts in some cases.
- (bugfix) Reinstate check for random 2 <nrand>s of 00.
- Always check triggers and bits in random 2s, not just if <nrand> is valid.
- (bugfix) Skip the offset byte after code 01 in strings.
- (bugfix) Report correct offsets when random 2s misuse the available bits.
- Complain about shift-and-add-mod when add%mod==0, not just when add==0.