FJ_CRE_VALIDITY and FJ_CRE_REINDEX should be defined as proper functions

[4Luke4]

Doc states that these functions can take some variables like do_message, do_reindex, etc.

But if you look at how these functions have been defined (here), you'll notice they can take no variables (i.e., do_message, do_reindex and the like are not local to the functions).

I suggest to code them as proper functions (even if the final result is the same). So if we consider FJ_CRE_VALIDITY as an example:

DEFINE_PATCH_FUNCTION ~FJ_CRE_VALIDITY~
INT_VAR
  do_message = 0
  do_reindex = 1
  do_eff = 1
RET
  valid
BEGIN
  SPRINT m1 ~is corrupt~
  SPRINT m2 ~below minimum length~
  SPRINT m3 ~header misplaced~
  SPRINT m4 ~extended structures point to header~
  SPRINT sg ~CRE V1.0~
  valid = 1
  PATCH_IF ~%SOURCE_RES%~ STRING_EQUAL_CASE charbase BEGIN
    valid = 0
  END ELSE BEGIN
    PATCH_IF BUFFER_LENGTH < 0x2d4 BEGIN
      valid = 0
      PATCH_IF do_message THEN BEGIN
        PATCH_PRINT ~%SOURCE_FILE% %m1%: %m2%.~ //is corrupt: below minimum length
      END
    END ELSE BEGIN
      READ_ASCII 0 sg
      PATCH_IF ~%sg%~ STR_CMP ~CRE V1.0~ BEGIN
        valid = 0
        PATCH_IF do_message THEN BEGIN
          PATCH_PRINT ~%SOURCE_FILE% %m1%: %m3%.~ //is corrupt: header misplaced
        END
      END ELSE BEGIN
        DEFINE_ASSOCIATIVE_ARRAY cre_offset BEGIN
          0x2a0 => 0x2a4
          0x2a8 => 0x2ac
          0x2b0 => 0x2b4
          0x2b8 => 0x2c0
          0x2bc => 0x2c0
          0x2c4 => 0x2c8
        END
        PHP_EACH cre_offset AS tmp => tmp_1 BEGIN
          READ_LONG tmp_0 tmp_2
          READ_LONG tmp_1 tmp_3
          PATCH_IF tmp_3 = 0 && tmp_2 < 0x2d4 BEGIN
            WRITE_LONG tmp_0 0x2d4
          END
          PATCH_IF tmp_3 != 0 && tmp_2 < 0x2d4 BEGIN
            valid = 0
            PATCH_IF do_message THEN BEGIN
              PATCH_PRINT ~%SOURCE_FILE% %m1%: %m4%.~ //is corrupt: extended structures point to header
            END
          END
        END
      END
    END
  END

  PATCH_IF valid THEN BEGIN
    LAUNCH_PATCH_FUNCTION ~FJ_CRE_REINDEX~
    INT_VAR
      do_reindex
      do_eff
    END
  END
END

[4Luke4]

Just to clarify further:

If you run the following mod

BACKUP ~test/backup~
SUPPORT ~~

MODDER fun_args fail

BEGIN "Missing fun_args"

COPY_EXISTING "ACCALIA.CRE" "override" // Or any other existing CRE file...
    LPF FJ_CRE_VALIDITY
    INT_VAR
        do_message = 1
    END
BUT_ONLY

you will get this message: ERROR: Function argument [do_message] is not part of function definition.

[FredrikLindgren]

Will fix.

[4Luke4]

Will fix.

To be precise, the same holds for fj_are_structure. When you add a new container,

LPF fj_are_structure
    INT_VAR
    fj_type        = 8 //nonvisible
    fj_loc_x       = 4388
    fj_loc_y       = 2876
    fj_box_left    = 4372
    fj_box_top     = 2826
    fj_box_right   = 4420
    fj_box_bottom  = 2858
    fj_trap_loc_x  = 4380
    fj_trap_loc_y  = 2870
    fj_vertex_0    = 4411 + (2858 << 16)
    fj_vertex_1    = 4372 + (2845 << 16)
    fj_vertex_2    = 4382 + (2826 << 16)
    fj_vertex_3    = 4420 + (2839 << 16)
    STR_VAR
    fj_structure_type = container
    fj_name           = ~Cornerstone~
END

you will get the aforementioned error because variables like fj_vertex_0, fj_vertex_1 and the like are not part of function definition (it is fj_vertex_idx that is part of function definition).

Anyway, the situation is different this time because you cannot know a priori how many vertices you are going to define. As a result, it would be better to edit documentation in order to state that such variables are NOT part of function definition and must be defined before launching fj_are_structure, i.e.:

PATCH_WITH_SCOPE BEGIN
  fj_vertex_0 = 4411 + (2858 << 16)
  fj_vertex_1 = 4372 + (2845 << 16)
  fj_vertex_2 = 4382 + (2826 << 16)
  fj_vertex_3 = 4420 + (2839 << 16)

  LPF fj_are_structure
    INT_VAR
    fj_type        = 8 //nonvisible
    fj_loc_x       = 4388
    fj_loc_y       = 2876
    fj_box_left    = 4372
    fj_box_top     = 2826
    fj_box_right   = 4420
    fj_box_bottom  = 2858
    fj_trap_loc_x  = 4380
    fj_trap_loc_y  = 2870
    STR_VAR
    fj_structure_type = container
    fj_name           = ~Cornerstone~
  END
END

[FredrikLindgren]

Done in 9a51c33d9352c6ccdb8d259dc7d9bc3516dbf1de. This is not backward-compatible with

OUTER_SET do_message = 1
...
LPF FJ_CRE_VALIDITY END

which did work before but will not work after this change. Possibly a better solution would have been do declare a new interface and deprecate the old one.

[FredrikLindgren]

So I did change my mind, reverted 9a51c33 and added wrapper functions instead.