Author Topic: ADD_CRE_EFFECT (Read 12308 times)

the bigg · « **Reply #25 on:** March 04, 2011, 08:49:16 PM »

You guys decide what DLTCEP will consider consistent or valid in the next 37 hours and 23 minutes and I'll comply with the flavor du jour.

Miloch · « **Reply #26 on:** March 04, 2011, 11:35:01 PM »

Wisp could probably come up with something better (surprised he didn't) but this appears to handle both such cases (tested on kobsha01 and korgan11):

Code: [Select]

    //correcting offsets
    PATCH_FOR_EACH ___#offset IN 0x2a0 0x2a8 0x2b0 0x2b8 0x2bc BEGIN
      READ_LONG ___#offset ___#current_off
      PATCH_IF (___#offset = 0x2b8) BEGIN
        SET ___#current_count = 38
      END ELSE BEGIN
        READ_LONG (___#offset + 4) ___#current_count
      END
      PATCH_IF (___#current_count > 0) AND (___#current_off >= ___#fx_off) BEGIN
        WRITE_LONG ___#offset (___#current_off + 0x108)
      END
    END

I'll be testing this with a revised L1NPCs on BWP somewhat soonish, so I guess that's one way to find out if it breaks anything.

the bigg · « **Reply #27 on:** March 05, 2011, 04:31:23 AM »

I'll wait to hear back, then.

Miloch · « **Reply #28 on:** March 06, 2011, 05:16:42 AM »

This seems to work on the handful of NPCs I've used it on so far. I'm still using a local version though - I didn't know if you'd changed it in the beta.

the bigg · « **Reply #29 on:** March 06, 2011, 07:18:32 AM »

Added.

Wisp · « **Reply #30 on:** March 06, 2011, 09:11:42 AM »

This code will break files where off > fx_off and count = 0, because the offset should be updated but isn't.

May I suggest the code be reverted to the way it was and then left that way? There is nothing wrong with non-standard orders, whiny BS from DLTCEP notwithstanding.

the bigg · « **Reply #31 on:** March 06, 2011, 09:37:34 AM »

Quote from: the bigg on March 04, 2011, 08:49:16 PM

You guys decide what DLTCEP will consider consistent or valid in the next 37 hours and 23 minutes and I'll comply with the flavor du jour.

GeN1e · « **Reply #32 on:** March 06, 2011, 05:36:55 PM »

According to the changelog, since 225 the FJ_CRE_VALIDITY (used by ADD_CRE_ITEM) also includes FJ_CRE_REINDEX, so the non-standart order is no longer an issue, and the offset list can be reverted back to update only the 'correct' ones

Code: [Select]

		    //correcting offsets
		//    PATCH_FOR_EACH ___#offset IN 0x2a0 0x2a8 0x2b0 0x2b8 0x2bc BEGIN
		    PATCH_FOR_EACH ___#offset IN 0x2b8 0x2bc BEGIN //item num and islot
		      READ_LONG ___#offset ___#current_off
		      WRITE_LONG ___#offset (___#current_off + 0x108)
		    END

PS And 0x30 for BG1 version.
PPS and also for DELETE_CRE_EFFECT.

the bigg · « **Reply #33 on:** March 06, 2011, 05:46:47 PM »

Done. The next person to complain about anything in g_macros.tpa gets to compile the next official version of WeiDU and deal with the people who can't suddenly run it anymore.

GeN1e · « **Reply #34 on:** March 06, 2011, 05:49:15 PM »

I complain about CRLFs in it not displaying properly for me.

the bigg · « **Reply #35 on:** March 06, 2011, 06:21:45 PM »

Have fun looking for the correct version of tk.

Miloch · « **Reply #36 on:** March 06, 2011, 06:46:12 PM »

Quote from: Wisp on March 06, 2011, 09:11:42 AM

This code will break files where off > fx_off and count = 0, because the offset should be updated but isn't.

May I suggest the code be reverted to the way it was and then left that way? There is nothing wrong with non-standard orders, whiny BS from DLTCEP notwithstanding.

No it shouldn't get updated actually - at least not any of the vanilla files I've seen. And it's not actually a 'nonstandard order' as such - the offset is getting updated when it shouldn't be. That's the only issue. And yes, there is something wrong with that when several mods affect the same CREs - all it takes is one to mess something up (as we have seen with L1NPCs recently).

If you have a counterexample then give it (i.e. an actual filename that 'breaks'). Especially if you're going to suggest a kneejerk code regression without even trying for a better solution

.

Edit: FJ_CRE_VALIDITY/REINDEX doesn't fix this, or I wouldn't've reported it in the first place.

GeN1e · « **Reply #37 on:** March 06, 2011, 08:21:22 PM »

Non-standart order - when starting offsets are positioned differently from what is considered 'normal', e.g. effects going before headers in SPL/ITM.

The problem about ADD_CRE_ macro was it's inability to handle properly NSO files, since it was assuming they were SO and didn't bother to check if other offsets were in need of shifting.

As a solution, ADD_CRE began checking for all other offsets, thus allowing for NSO. A slight drawback of it was the possibility of it itself creating NSO from SO, under certain conditions.

Since then, apparently it's been decided somewhere to use a full validity/reindex check before doing the specified patch. Therefore, the offset-checking became obsolete and no longer required to support NSO, but it didn't cease creating NSO on it's own.

PS I suppose some live testing wouldn't hurt then, to be sure.
PPS Curse it, I'll test atm.

GeN1e · « **Reply #38 on:** March 06, 2011, 09:08:52 PM »

Alright then.

That same poor inconsistent HDRAGSIL, that crashed on me months ago after ADD_CRE_EFFECT. Also trying to add an eff to a zeroed out cre.

1) old macro without offset-check
2) old macro with offset-check
3) new macro with offset-check
4) new macro without offset-check

HDRAGSIL:
1) DLTCEP, crash
2) DLTCEP
3) fine
4) fine

ZEROED:
1) fine
2) DLTCEP
3) DLTCEP
4) fine

Miloch · « **Reply #39 on:** March 07, 2011, 03:03:20 AM »

Was hdragsil corrupted by some mod (Quest Pack or something)? Because the vanilla version is fine after running the code I posted, even without validity/reindexing.

Do you have a vanilla example of a 'zeroed out cre'? I'll admit, I'm not quite sure why zeroing out some offsets is a good idea (I think it was a Taimon utility that did this - I'm assuming he had a good reason for it).

I tested my code on various CREs with differing numbers of effs/items/spell counts etc. and *without* running the validity/reindexing with no issues so far. But it seems tough to find many nonstandard CREs these days, since just about every mod and WeiDU function includes the reindex code.

Incidentally, is there still a way to pass a do_message = 1 to FJ_CRE_VALIDITY if it's called inside another function or macro?

the bigg · « **Reply #40 on:** March 07, 2011, 03:39:49 AM »

Quote from: Miloch on March 07, 2011, 03:03:20 AM

Incidentally, is there still a way to pass a do_message = 1 to FJ_CRE_VALIDITY if it's called inside another function or macro?

Would this work?

Code: [Select]

--- a/src/tph/include/fj_cre_validity.tpa
+++ b/src/tph/include/fj_cre_validity.tpa
@@ -1,4 +1,5 @@
-DEFINE_PATCH_FUNCTION ~FJ_CRE_VALIDITY~ INT_VAR do_message = 0 RET valid BEGIN
+DEFINE_PATCH_FUNCTION ~FJ_CRE_VALIDITY~ RET valid BEGIN
+  SET do_message = IS_AN_INT do_message ? 0 : do_message
   SPRINT m1 ~is corrupt~
   SPRINT m2 ~below minimum length~
   SPRINT m3 ~header misplaced~

Then you can LPF ~ADD_CRE_ITEM~ INT_VAR do_message = 1 opcode = 53 blah blah blah END and do_message = 1 should still be visible by FJ_CRE_VALIDITY.

Miloch · « **Reply #41 on:** March 07, 2011, 03:53:45 AM »

That would be great. Ordinarily, I would be fine with not seeing the messages but now (during testing) I'm specifically looking for corrupt/nonstandard CREs. (The problem is, I don't think that will identify CREs hosed *after* modding them, but it's a start at least.)

Come to think of it, I might want to run FJ_CRE_VALIDITY on all CREs after installing something like L1NPCs, with the messaging and *without* reindexing them, just to see if we botched anything. Can we do that or does F_C_V automatically invoke F_C_R no matter what?

the bigg · « **Reply #42 on:** March 07, 2011, 03:57:05 AM »

For now it always run FJ_CRE_REINDEX and FJ_CRE_EFF_Vx. I guess I could add do_reindex and do_eff, which default at 1 but can be set to 0 to skip the relevant function:

Code: [Select]

diff --git a/src/tph/include/fj_cre_validity.tpa b/src/tph/include/fj_cre_validity.tpa
index d61cb81..dfa71fd 100755
--- a/src/tph/include/fj_cre_validity.tpa
+++ b/src/tph/include/fj_cre_validity.tpa
@@ -1,4 +1,5 @@
-DEFINE_PATCH_FUNCTION ~FJ_CRE_VALIDITY~ INT_VAR do_message = 0 RET valid BEGIN
+DEFINE_PATCH_FUNCTION ~FJ_CRE_VALIDITY~ RET valid BEGIN
+  SET do_message = IS_AN_INT do_message ? do_message : 0
   SPRINT m1 ~is corrupt~
   SPRINT m2 ~below minimum length~
   SPRINT m3 ~header misplaced~
@@ -52,6 +53,8 @@ DEFINE_PATCH_FUNCTION ~FJ_CRE_VALIDITY~ INT_VAR do_message = 0 RET valid BEGIN
 END
 
 DEFINE_PATCH_FUNCTION ~FJ_CRE_REINDEX~ BEGIN
+  SET do_eff = IS_AN_INT do_eff ? do_eff : 1
+  SET do_reindex = IS_AN_INT do_reindex ? do_reindex : 1
   fv = 0
   kso = 0x2d4
   ksc = 0
@@ -76,12 +79,13 @@ DEFINE_PATCH_FUNCTION ~FJ_CRE_REINDEX~ BEGIN
   SPRINT il fail
   SPRINT el fail
   READ_BYTE 0x33 fv //EFF version
-  PATCH_IF (fv = 0) AND NOT (FILE_EXISTS_IN_GAME ~ar0125.are~) BEGIN
+  PATCH_IF (fv = 0) AND NOT (FILE_EXISTS_IN_GAME ~ar0125.are~) && do_eff BEGIN
     LAUNCH_PATCH_FUNCTION ~FJ_CRE_EFF_V2~ END
   END
-  PATCH_IF (fv != 0) AND (FILE_EXISTS_IN_GAME ~ar0125.are~) BEGIN
+  PATCH_IF (fv != 0) AND (FILE_EXISTS_IN_GAME ~ar0125.are~) && do_eff BEGIN
     LAUNCH_PATCH_FUNCTION ~T-CRE_EFF_V1~ END
   END
+  PATCH_IF do_reindex BEGIN
   READ_BYTE 0x33 fv //EFF version
   PATCH_IF FILE_EXISTS_IN_GAME ~fw0125.are~ BEGIN //Fix buggered Tutu refs
     FOR (s1 = 0xa4; s1 < 0x234; s1 += 0x4) BEGIN
@@ -203,6 +207,7 @@ DEFINE_PATCH_FUNCTION ~FJ_CRE_REINDEX~ BEGIN
     END
   END
   SOURCE_SIZE	 = off_0 + 0x50
+  END
 END
 
 DEFINE_PATCH_FUNCTION ~FJ_CRE_EFF_V2~ BEGIN

Wisp · « **Reply #43 on:** March 07, 2011, 04:37:14 AM »

Quote from: Miloch on March 07, 2011, 03:03:20 AM

Was hdragsil corrupted by some mod (Quest Pack or something)? Because the vanilla version is fine after running the code I posted, even without validity/reindexing.

Do you have a vanilla example of a 'zeroed out cre'? I'll admit, I'm not quite sure why zeroing out some offsets is a good idea (I think it was a Taimon utility that did this - I'm assuming he had a good reason for it).

QP overwrites with a cre with a zeroed offset.
Taimon's code fixed zeroed offsets, if I'm not mistaken.
Zeroing offsets is a stupid thing to do and is hopefully not done on purpose.

Miloch · « **Reply #44 on:** March 08, 2011, 03:52:01 PM »

Quote from: the bigg on March 07, 2011, 03:57:05 AM

I guess I could add do_reindex and do_eff, which default at 1 but can be set to 0 to skip the relevant function

Cool. If you want to push a new beta with that, I'll run it on L1NPCs and Aurora (maybe even a full BWP if I get the time/motivation).

the bigg · « **Reply #45 on:** March 08, 2011, 04:51:37 PM »

Done.

the bigg · « **Reply #46 on:** March 11, 2011, 03:59:40 AM »

Any update on your testing? I think Aurora + L1NPCs would be enough, no need for a full BiG.

Miloch · « **Reply #47 on:** March 11, 2011, 10:26:17 AM »

Should be done in a few hours. I'm not going to bother with BWP diffs then, if you don't need them. One BWP eats up enough space.

the bigg · « **Reply #48 on:** March 11, 2011, 10:29:42 AM »

L1NPCs installing correctly is cause to assume that all DO_THIS_TO_CRE function properly (and Gen1e above reported that this macro doesn't barf on the known-broken D0QP).

Miloch · « **Reply #49 on:** March 11, 2011, 02:56:49 PM »

Both mods seem to install fine on Tutu and BWP with v228.12. Though I found out the hard way it is probably pretty stupid to leave MODDER enabled when testing on BWP.

News:

Author Topic: ADD_CRE_EFFECT (Read 12308 times)