How to use ReDefine

[burner1024]

Hi. I couldn't find a separate repo for ReDefine, so I guess this is the only place where I can ask...

I'd like to try ReDefine for UP/RP, in order to reduce code volume further. As far I understand, it automatically processes headers, and replaces what's possible. That looks nice, but I have a few questions (and that's just getting started):

1. It appears that it does some sort of autoformat. Is it possible to do that separately? Unified code style is good, but I'll like to be able to inspect actual code change, and including whitespace just shows that the whole file is different.
2. How safe is it? In terms of silently doing a wrong thing which then gets compiled and shipped?
3. Why multiple macros are specified expllicitly in the config? Does it replace only those specified?
4. What do I to with this, for example:
   WARNING (IfArgumentIs) function<critter_add_trait> must be added to configuration before using this action : fileline<arroyo\acbrahmn.ssl:128> :: critter_add_trait(self_obj,TRAIT_OBJECT,OBJECT_TEAM_NUM,TEAM_ARROYO);
5. Reasonably, if run on UP/RP codebase without any changes, how well do you think it'll work? What would probably need adjustment/tailoring?

ref BGforgeNet/Fallout2_Unofficial_Patch#33

[wipe2238]

1. Autoformating is just a side-feature, taking advantage of how each lines are processed. For functions, it can be disabled by setting [ReDefine]->FormatFunctions=0, which will try to preserve original format when editing function. It wasn't heavily tested, so there might be some glithes with it :(
   Removing [ReDefine]->FormatFunctionsForced will keep original formatting when not editing function. Other values (plus tiny examples) can be found here.
   There is few micro-changes which cannot be turned off, but they are disabled if there is no real changes to code.

2. We've been running ReDefine over every single script (with autocommits when needed) everyday since... half year? Something like that. But it took hundreds of dry runs (--read-only option) and checking thousands of lines first; at some point it felt weird to run program without --read-only, that's how used to it i was :) I'd recommend same approach every time you introduce some bigger change. Once you think it's perfect, make a dry run one more time. Or two. Or three, just to be sure. @Lexx2k would get dozens of heart attacks if he knew how many times it saved mod from disaster.
   But that's about manual changes, before GitHub repo even existed.
   Nowadays, every commit it tested twice - first test is to compile scripts just as they've been commited, second test is to run ReDefine and compile them again. If some of changes introduced by ReDefine break scripts (for example changing code to use macro from not included file), commiter gets email from CI and have a chance to fix it before ReDefine changes are commited to repo (daily at 5am; and if somehow scripts are still broken at that time, nothing will be commited at all).
   So yeah, trusting in ReDefine config to be correct is one thing, protecting repo from broken autocommits is another.

3. Yeah. Way how defines/macros are used in Fallout2 modding is a huge nightmare fuel, i wouldn't dare to try outsmart two decades of changes in headers. Even sfall adds some ugliness here and there, so it's pointless to fight it. If something is not declared in config, it's never used.

4. You missed base config for critter_add_trait under [Function]
   https://github.com/rotators/Fo1in2/blob/86a331eba5f514bc6f2f564042d173cbd0e89d9b/Tools/ReDefine/ReDefine.cfg#L122
   It also needs all related defines to be added to config (TRAIT, OBJECT, TEAM, AI, etc.). Keep in mind that there are two groups of defines using same prefix (TRAIT) - you'll either have to move one group to another header or change one group's prefix to something else.
   Also, some "scripted" replacement actions (here - IfArgumentIs) might require function/variable to be added to either [Function] or [Variable] to work properly.

5. We put quite some effort to make 1@2 code absurdly clean, just so ReDefine could run without any warnings or hiccups. As UP/RP comes from Fallout2 scripts... there will be warnings. MANY warnings. You'll get warnings generated by comments even, at least i did when running over "clean" Fallout2 scripts.
   However, program always gives up on applying any changes if it spots something not looking as expected. It would tweak what it can fully understands, skipping everything else.
   Things to remember - program has no clue about /* long comments */ - if line doesn't start with //`, it's a code and will be processed. That might lead to some screwups, especially in headers which contains block(s) of some old, commented defines. You know it's unused. Compiler knows it's unused. ReDefine doesn't :(
   Headers are most important thing here, so they should be taken care of until there's not a single warning generated by them. And some of them might be considered annoying - ReDefine is not a parser/preprocessor, even if it should be. It's just regex on heavy drugs.
   I can't predict what might

As you see in only active ticket [except this one], ReDefine heavily lacks any real documentation (shocking, isn't it?) or any practical examples... i got lazy with whole thing as there was always something to fix/tweak in 1@2 :) Guess it's time to finally write proper documentation and release damn thing - so questions are welcome, i'll know on what parts to focus first.

[burner1024]

Re-compiling and comparing is a good idea, not sure why I didn't think about it right away. I did the same when testing optimization options for UP/RP.
But excluding multiline comments seems like a simple enough thing, no? I mean, it's just a short regex.
I'm not sure I'll need autoruns, for now I'm looking for a one-time cleanup. UP/RP aren't under heavy development like 1@2. That may change, of course.

I'm not getting this about critter_add_trait. The config is exactly the one from 1@2 repo, just paths changed. What else it needs?

Also, does it re-process until there's a definitive version, or running it again might result in a yet tighter code?

And, it appears to convert everything to windows crlf explicitly, is there a way to disable that?

[wipe2238]

What else it needs?

Actual TRAIT defines (TRAIT_gifted, TRAIT_small_frame, etc.) must be in header which does not have TRAIT_PERK/TRAIT_OBJECT/TRAIT_TRAIT defines.
Program grabs defines by checking prefix (in our setup - TRAIT) and those two groups sharing same prefix and being in same header = program gets confused what is what. Putting them in 2 different files is simplest way to make it work (lucky, it's only case when moving defines somewhere else is required).

does it re-process until there's a definitive version, or running it again might result in a yet tighter code?

That depends on how complex [Script] sections are and how they're configured. 1@2 setup aims at most "packed" code, and uses a few tricks to achieve that in a single run. Some changes have different priority than others, some changes are in specific order, and in few cases [Script] setup forces processing current line again from scratch (after applying current changes), so previously skipped edits have a chance to run.

it appears to convert everything to windows crlf explicitly, is there a way to disable that?

[ReDefine]->UnixNewlines=true should do the trick.

[burner1024]

[ReDefine]->UnixNewlines=true should do the trick.

It doesn't.

But otherwise, after separating TRAIT_ defines and adding #define PID_NONE (-1) to itempid.h, there's a surprisignly little amount of warnings left. Well, I've been shoveling dung for almost a year now, so I guess that pays off.

Still lots of debug, though. I suppose it needs to be cleaned as well?

DEBUG (ReDefine::TextGetFunctions) FUNCTION(0) : fileline<valtcity\vcrandal.ssl:247> :: if( ( local_var( LVAR_Randal_Status ) == RANDAL_CONVINCED ) or
DEBUG (ReDefine::TextGetFunctions) IGNORED(1) : fileline<valtcity\vcrandal.ssl:247> :: if( ( local_var( LVAR_Randal_Status ) == RANDAL_CONVINCED ) or
DEBUG (ReDefine::TextGetFunctions) 	calc[if( ( local_var( LVAR_Randal_Status ) == RANDAL_CONVINCED ) or]
DEBUG (ReDefine::TextGetFunctions) 	full[] b=1
DEBUG (ReDefine::TextGetFunctions) 	func[if] args[|( local_var( LVAR_Randal_Status ) == RANDAL_CONVINCED ) or] op[] opArg[] 

What's this saying, what should be done about it?

[wipe2238]

It doesn't.

Huh, weird. Ran over UP/scripts_src/ with 1@2 config and only UnixNewlines added, looks LF is used in edited files as it should. Are you using compiled ELF or .exe from 1@2 repo? I'm not sure .exe was updated after adding this option :/
In second case - ReDefine was in 90% developed/tested on *nix, so there's really no reason to stick to wine+exe; ./Build.sh should take care of everything needed to get native version, as long everything is installed (cmake, g++, any other standard stuff required to compile something).
DERP. I was looking at local repo all the time where this change is added, but was never pushed since few months... Sorry for fake news :( it should work now.

What's this saying, what should be done about it?

It failed to properly extract function from given line, as it thinks if is a function... lot of () and many operators after function (or, and, both) is way to complex for not-a-parser. I'll add a tiny check so it won't treat if() as a function, which should help a bit.

If you have same warning with regular function(s) giving "ignored" warnings and don't really want to mess with them, adding space between name and ( will make it invisible to ReDefine :P SSL compiler can handle some_func () perfectly, and it might save you from debug spam.

[burner1024]

I was using Windows version from 1@2: I didn't know this repo existed. Using a native version is certainly more pleasant.

With latest master compiled, newlines work properly and debug size is reduced considerably. The last bulk source of clutter are debug messages:

FUNCTION(1) : fileline<den/dcjoey.ssl:396> :: ndebug("gave_locket == "+gave_locket+" / get_anna_locket_pip(anna_locket_pip_assigned) == "+get_anna_locket_pip(anna_locket_pip_assigned)+" / mom_fingered_joey == "+mom_fingered_joey);
DEBUG (TextGetFunctions) IGNORED(Q) : fileline<den/dcjoey.ssl:396> :: ndebug("gave_locket == "+gave_locket+" / get_anna_locket_pip(anna_locket_pip_assigned) == "+get_anna_locket_pip(anna_locket_pip_assigned)+" / mom_fingered_joey == "+mom_fingered_joey);
DEBUG (TextGetFunctions)        calc[get_anna_locket_pip(anna_locket_pip_assigned) == "+get_anna_locket_pip(anna_locket_pip_assigned)+" / mom_fingered_joey == "+mom_fingered_joey);]
DEBUG (TextGetFunctions)        full[get_anna_locket_pip(anna_locket_pip_assigned) == "+get_anna_locket_pip(anna_locket_pip_assigned)+" / mom_fingered_joey == ] b=0
DEBUG (TextGetFunctions)        func[get_anna_locket_pip] args[|anna_locket_pip_assigned] op[==] opArg["+get_anna_locket_pip(anna_locket_pip_assigned)+" / mom_fingered_joey == "+mom_fingered_joey);]

ndebug is #define ndebug(message) debug_msg(SCRIPT_REALNAME + ": " + message + "\n")
I searched the config for "text", "debug" - there isn't much. How to get rid of this?

Also, one thing I just realized: scripts don't actually need to be compiled for comparing. Just preprocessing should be enough. In fact, it could probably be built directly into ReDefine, something like preprocessor_cmd = gcc -E -x c -P. Or it could be a helper script shipped alongside.

[wipe2238]

ndebug is fine (obviously), it's function-like stuff inside "" which is confusing :S You could either change it slightly different format or add /*ReDefine::IgnoreLine*/" anywhere, so program won't even start processing given line.

Thinking of that, maybe i should disable debug log for function extraction, and enable it only on demand. There's not much which can be done with it right now, other than various way of hiding it.

I'm not sure what changes you want to catch; as long you don't have [ReDefine]->FormatFunctionsForced = true set, ReDefine.log should be fine to grab real changes, see https://github.com/rotators/Fo1in2/runs/477962933#step:4:1 Microchanges won't be applied if there's no @@ entry for given file.
If that's not enough, i had some bash script once which automagically generates .diff files when needed; most likely i have it stashed somewhere.

[burner1024]

Thinking of that, maybe i should disable debug log for function extraction, and enable it only on demand. There's not much which can be done with it right now, other than various way of hiding it.

If that's the case, it's probably a good idea.

I'm not sure what changes you want to catch; as long you don't have [ReDefine]->FormatFunctionsForced = true set, ReDefine.log should be fine to grab real changes, see https://github.com/rotators/Fo1in2/runs/477962933#step:4:1 Microchanges won't be applied if there's no @@ entry for given file.

The changes that give heart attacks. I mean, those that actually alter the scripts rather than re-use defines.

[burner1024]

Looks like it uses macros directly from config, doesn't even check whether they are defined in the headers?

@@ -272,8 +272,8 @@
 end
 procedure Node2217a begin
    if ((item_caps_total(dude_obj)) >= 100) then begin
-      item_caps_adjust(dude_obj, -100);
-      item_caps_adjust(self_obj, 100);
+      dude_caps_adjust( -100);
+      self_caps_adjust( 100);
       critter_heal(dude_obj, -((get_critter_stat(dude_obj,(35))) - (get_critter_stat(dude_obj,(7)))));
       call Node022;
    end else begin

The above example could smarter about whitespace. While the one below could be dumber (no reason to change).

       if (obj_carrying_pid_obj(dude_obj, (17))) then
-         item:=obj_carrying_pid_obj(dude_obj,(17));
+         item:=obj_carrying_pid_obj(dude_obj, (17));
       else if (obj_carrying_pid_obj(dude_obj, (381))) then
-         item:=obj_carrying_pid_obj(dude_obj,(381));
+         item:=obj_carrying_pid_obj(dude_obj, (381));

[wipe2238]

Looks like it uses macros directly from config, doesn't even check whether they are defined in the headers?

Yes. Headers are used only to get values for arguments, and nothing more.
That allows quick changing script written by someone else (like this one) to macros you already know and love, leaving you with only includes to fix.
Second, more important reason is that ReDefine shines brightest when working on decompiled scripts, which - obviously - have no #includes anywhere. Adding requirement to have macro defined before editing line would add lot of complexity :/

The above example could smarter about whitespace. While the one below could be dumber (no reason to change).

That's with FormatFunctions=0, right? I'll look into that.

[burner1024]

That's with FormatFunctions=0, right? I'll look into that.

right

[wipe2238]

Can you link to script from 2nd diff?

[burner1024]

fcdrfung

Actually, a non-preprocessed version looks good:

@@ -666,14 +666,14 @@ procedure NodeDoDermalUpgrade begin
    variable item;
 
    if (dude_caps > 33000 and (dude_item(PID_COMBAT_ARMOR) or dude_item(PID_COMBAT_ARMOR_MK_II) or dude_item(PID_BROTHERHOOD_COMBAT_ARMOR))) then
 begin
-      item_caps_adjust(dude_obj,-33000);
+      dude_caps_adjust(-33000);
       if (dude_item(PID_COMBAT_ARMOR)) then//added check - killap
-         item:=obj_carrying_pid_obj(dude_obj,PID_COMBAT_ARMOR);
+         item:=dude_item(PID_COMBAT_ARMOR);
       //added by killap
       else if (dude_item(PID_COMBAT_ARMOR_MK_II)) then
-         item:=obj_carrying_pid_obj(dude_obj,PID_COMBAT_ARMOR_MK_II);
+         item:=dude_item(PID_COMBAT_ARMOR_MK_II);
       else if (dude_item(PID_BROTHERHOOD_COMBAT_ARMOR)) then

It's the preprocessed one that's different. I assume that's due to differences in ReDefine.cfg vs actual headers?

[wipe2238]

Previously preprocessor replaced just PID so everything was at exactly same positions. Now it has to insert whole macro, including space between arguments :)

[burner1024]

OK, I'll just run compilation for comparing instead. It takes longer, but whatever.