Wrong decompilation of nested if in else blocks #2

Activity

1. auronen

   added

   bugSomething isn't workingSomething isn't working

   on Jun 30, 2022
2. auronen

   added a commit that references this issue on Jun 30, 2022

   dirty fix for #2

   99820f0
3. auronen

   added

   help wantedExtra attention is neededExtra attention is needed

   on Jul 9, 2022
4. lmichaelis commented on Sep 11, 2024

   

   lmichaelis

   on Sep 11, 2024

   Last edited by lmichaelis

   More actions

   @auronen good news: I thought of a solution. I've got a new decompiler in progress which produces the correct output (minus the broken floats, ignore those :D):

   func void B_SETNPCVISUAL(var C_NPC SLF, var int GENDER, var string HEADMESH, var int FACETEX, var int BODYTEX, var int ARMORINSTANCE) {
   	SLF.AIVAR[17] = GENDER;
   	MDL_SETVISUAL(SLF, "HUMANS.MDS");
   	if (GENDER == MALE) {
   		MDL_SETVISUALBODY(SLF, "hum_body_Naked0", BODYTEX, 0, HEADMESH, FACETEX, 0, ARMORINSTANCE);
   		if (SLF.ATTRIBUTE[4] < 50) {
   			MDL_SETMODELSCALE(SLF, 1063675494, 1065353216, 1065353216);
   		};
   		if (SLF.ATTRIBUTE[4] > 100) {
   			MDL_SETMODELSCALE(SLF, 1066192077, 1065353216, 1065353216);
   		};
   	} else {
   		if (BODYTEX >= 0 && BODYTEX <= 3) {
   			BODYTEX = BODYTEX + 4;
   		};
   		MDL_SETVISUALBODY(SLF, "Hum_Body_Babe0", BODYTEX, 0, HEADMESH, FACETEX, 0, ARMORINSTANCE);
   	};
   	return;
   }
   

   The trick? Don't try to parse else if. Instead, when hitting a branch if zero, decompile the first branch as a block, until either the target address of the initial branch if zero is hit, or an unconditional branch is hit. Then report back to the caller if an unconditional branch was hit and where its jump destination is.

   In the caller (where you initially hit the branch), check if the return indicates an unconditional branch. If it does, treat the code between the initial branch if zero target and the unconditional branch target of the first block as the 'else' part of that if statement and parse it the same way, this time ignoring the return value.

   This way you always get

   if (...) {
      ...
   } else {
      if (...) {
         ...
      } else {
         if (...) {
         };
      };
   };
   

   instead of

   if (...) {
   } else if (...) {
   } else if (...) {
   };
   

   but the logic will always be correct, even if it might look ugly. As a post-processing step it would be possible to collapse these nested if-else statements too.

   I hope this explanation makes sense :)
5. auronen commented on Oct 13, 2024

   

   auronen

   on Oct 13, 2024

   OwnerAuthor

   More actions

   Thank you, @lmichaelis, for the response.
   This is exactly what my drity fix 99820f0 is doing. It produces this output:

   func void b_setnpcvisual(var c_npc slf, var int gender, var string headmesh, var int facetex, var int bodytex, var int armorinstance) {
       slf.aivar[17] = gender;
       mdl_setvisual(slf, "HUMANS.MDS");
       if (gender == male) {
           mdl_setvisualbody(slf, "hum_body_Naked0", bodytex, 0, headmesh, facetex, 0, armorinstance);
           if (slf.attribute[atr_strength] /*4*/ < 50) {
               mdl_setmodelscale(slf, 0.9, 1.0, 1.0);
           };
           if (slf.attribute[atr_strength] /*4*/ > 100) {
               mdl_setmodelscale(slf, 1.1, 1.0, 1.0);
           };
       }
       else {
           if ((bodytex >= 0) && (bodytex <= 3)) {
               bodytex = bodytex + 4;
           };
           mdl_setvisualbody(slf, "Hum_Body_Babe0", bodytex, 0, headmesh, facetex, 0, armorinstance);
       };
   };

   it was a dirty fix, but it is, as you say, correct. This ambiguity in the way the code can be written was really what bothered me back then... And still does today :D
   I wanted to have a neat decompilation where an else block with a single if would be represented as an else if. Today I would probably parse it back in and format it to be else if. But I don't think this sort of cosmetic feature is worth it.