NPCs fail to appear after writing new values to the packfile.

[ZoriaRPG]

For some reason, just adding the script types after edefSCRIPT and before edefLAST caused all of the values in the enemy editor to corrupt themselves. I'm not sure why.

Thus, instead of mucking with that table, I added an entirely new array for them/

Here's what I've done:

I added:

//zdefs.h
enum { scriptDEF1, scriptDEF2, scriptDEF3, scriptDEF4, criptDEF5, scriptDEF6, scriptDEF7, scriptDEF8, scriptDEF9, scriptDEF10, scriptDEFLAST };

in struct guydata, I added byte scriptdefense[scriptDEFLAST];

//guys.h
Likewise, I added public byte scriptdefense[scriptDEFLAST]; to the guydata struct.

//zq_class.cpp

To writeguys(), I added:



for(int j=0; j < scriptDEFLAST; j++)
{
if(!p_putc(guysbuf[i].scriptdefense[j],f))
{
new_return(51);
}
}


I further tried this with a return of 50, disabling the misc12 writeout, to verify that it wasn't the return value.

//ffscript
I added all the required ZScript things to the ZScript files. That part does work and writing to, and reading from n->ScripDefense[10] works flawlessly.

//guys.cpp

I added:



for(int w=0; w < scriptDEFLAST; w++)
scriptdefense[w]=d->scriptdefense[w];


inside enemy::enemy(fix X,fix Y,int Id,int Clk) : sprite()

Finally, I worked on zq_custom.cpp

I added jwin stuff, which does seem to work:

Tables stuff:
-=SPOILER=-

..and in the edit_enemyedit() function:

-=SPOILER=-


What did I miss here?

I do not see anything that should prevent an npc from spawning out of these mods...?!

Yet...somehow when writing the data out to the pack file, all of the npc data is corrupted. Edit an enemy, and load a quest where it has been edited, saved, and reopened, and look at the npc tables.

In the test quest, if you click on the screen to freeze it, you can briefly see that ZC is trying to create an npc object, but can't. It does not return as a valid NPC, meaning that Screen->LoadNPC(1) returns invalid, and it never moves.

Source files, binaries, and a test quest. (http://timelord.insomnia247.nl/zc/zc_dev/ZC_2.54_Sources_Beta_24.zip)

If I disable the write-out (zq_class.cpp) though, and I otherwise edit an npc and save, it works as normal.

???

[Saffith]

You changed the saving code, but not the loading code. That's not going to go well.

[ZoriaRPG]

You changed the saving code, but not the loading code. That's not going to go well.

Fair enough...and where, in what file, is that located?

I was trying to figure out earlier if this was stored in a quest header definition, but I couldn't, and I still cannot find it?!

Edit: Oh, I see. qst.cpp

Do I need to do more than this:



if(guyversion >= 22)
{
if(!p_igetl(&(tempguy.misc11),f,keepdata))
{
return qe_invalid;
}

if(!p_igetl(&(tempguy.misc12),f,keepdata))
{
return qe_invalid;
}

for(int j=0; j<scriptDEFLAST; j++)
{
if(!p_getc(&(tempguy.scriptdefense[j]),f,keepdata))
{
return qe_invalid;
}
}
}


I also need to know what defines the 'guyversion' input, so that I can add a new minversion; but I guess I'll find out if this works first.

Well, that clearly isn't sufficient. I missed something else, as now I have junk data everywhere. Any ideas?

How sensitive are the guydata class and struct, regarding insertion points? I inserted scriptdefense[] at the end of the public decs, in the class (guys.h); and as the very last entry in the strucct in zdefs.h. I can't see this causing data corruption on this scale.

[Gleeok]

Oh, sweet dear little baby infant Jesus.....

Please, please, pleeeease, be extremely careful when changing packfile formats. Even I go into "serious mode" when doing anything with that file.

[ZoriaRPG]

Oh, sweet dear little baby infant Jesus.....

Please, please, pleeeease, be extremely careful when changing packfile formats. Even I go into "serious mode" when doing anything with that file.

it would be extremely helpful to know what in the world adding one variable is doing that is causing such massive corruption. Hell, why did expanding one enum do the same blasted thing?

Obviously I wouldn't retain anything for a release that is horribly broken, nor commit it to a main, but holy flidd, look at this nonsense:

http://timelord.insomnia247.nl/zc/zc_dev/ZC_2.54_Sources_Beta_29.zip <-- Code and binaries.

All I did was add in a tiny bit of code to read in, and write out script defs. It would be very nice to be able to set them from the enemy editor, and I know of no other way to do it. What exactly is it that I'm doing wrongly here?

When I increased the enum for the weapon defense types--which by its very nature, expanded the sizes of the arrays, and the functions that called and wrote to them--it did something very similar. All the values were corrupted, and things went bloody otherworldly.

How does one properly expand these things without the universe imploding?

All of my changes to the files associated with this are in that archive. Care to review?

This is driving me bonkers. Adding the array is easy. Adding entries to the editor is easy. Hell, I could even add in the weapon checks, and these values could be set up by script. ... but people want to use the enemy editor, and despite --AFAIK-- doing everything properly, it's going bonkers. I have no idea if I would ever need to go into this again, but if I don't, someone will want to do it, and we really need to know what's going on.

I'm certainly open to any 'better way' of handling it.

Should I just make an array in zdefs with byte scriptdefs[npcMax][defSCRIPTMAX] and write all the values to that? Wouldn't I need to save that out just the same?

[Gleeok]

Can you post just the code you changed in ZC (Not ZQuest; and not any script, ffc, or ZScript code)? I'll take a look.

If you comment out everything you added to save/load guys does the enemy data still get corrupted?

[ZoriaRPG]

Can you post just the code you changed in ZC (Not ZQuest; and not any script, ffc, or ZScript code)? I'll take a look.

If you comment out everything you added to save/load guys does the enemy data still get corrupted?

No, if I comment it out, then everything works. Setting the defence values by script works like a champ, too.

I'll post the relevant sections here, and I'll upload the ZIP.

zdefs.cpp
-=SPOILER=-

Guys.cpp
-=SPOILER=-

zq_class.cpp
-=SPOILER=-

[ZoriaRPG]

...continued...

qst.cpp
-=SPOILER=-

zq_custom.cpp
-=SPOILER=-

Search for 'scriptdefs' in those functions. The codeblocks that I commented out to prevent corruption are commented out in blocks.

Entire Package (http://timelord.insomnia247.nl/zc/zc_dev/ZC_2.54_Sources_Beta_30.zip)

As I said, the defs do work, when set by script. All of that is in place, save for an exception based on a quest version, to use the old checking method.

If you need to read the table for the jwin stuff, it's in there.

Once we get the data corruption sorted, this feature is effectively ready to use.

[Gleeok]

Okay, I just took a look at it all. *phew*

Fix #1: The saving code is corrupting the quest file, the loading code looks fine...-ish (see fix #2). Put the serializing of scriptdefenses[] after the serializing of guy->misc12.
Fix #2:


if(guyversion >= 23) // Add new guyversion conditional statement
{
for(int j=0; j<scriptDEFLAST; j++)
{
if(!p_getc(&(tempguy.scriptdefense[j]),f,keepdata))
{
return qe_invalid;
}
}
}


Fix #3: Save guysversion as the current version (23).

I don't see anything else right now. Try that and let me know if it works.

[ZoriaRPG]

Okay, I just took a look at it all. *phew*

Fix #1: The saving code is corrupting the quest file, the loading code looks fine...-ish (see fix #2). Put the serializing of scriptdefenses[] after the serializing of guy->misc12.
Fix #2:


if(guyversion >= 23) // Add new guyversion conditional statement
{
for(int j=0; j<scriptDEFLAST; j++)
{
if(!p_getc(&(tempguy.scriptdefense[j]),f,keepdata))
{
return qe_invalid;
}
}
}


Fix #3: Save guysversion as the current version (23).

I don't see anything else right now. Try that and let me know if it works.

Where do I change the defs for guyversion, and add the new version; or did you mean that 23 is current? I'd still need to know where to set that.

[Gleeok]

Where do I change the defs for guyversion, and add the new version; or did you mean that 23 is current? I'd still need to know where to set that.

V_GUYS in zdefs.h.

[ZoriaRPG]

V_GUYS in zdefs.h.


Oh, must have glossed over it. probably because I didn't know the current version, and it didn't match the known ID.

Anyhow, still massive data corruption:

http://timelord.insomnia247.nl/zc/zc_dev/beta31.png
The default bubble enemy, upon loading the base quest template with this patch.

qst.cpp

-=SPOILER=-

zq_class.cpp

-=SPOILER=-

IDK if you want to try to debug a binary, or do a build.

[ZoriaRPG]

Something that I did made it worse... Looks as if there is now corruption even with those two disabled. :(

The odd thing, is that I didn't touch anything related to those, but I made a slight modification of the item editor. Saving the quest, and reopening it somehow scrambled the data. I'm not sure how, at this point. As soon as the quest is saved, the data is corrupted, so I'm, rolling back again to beta 30.

(The beta 30 sources are stable.)

---------------

I rolled it back to 30, which still compiled and ran with no issues (packfile saving of the new array is disabled).

I tried reintroducing your changes, this time moving the loading sequence a bit farther down the line. No difference.

You should probably see the result for yourself.

Here's Beta 33, which has your suggested changes implemented:

http://timelord.insomnia247.nl/zc/zc_dev/ZC_2.54_Sources_Beta_33_Packfile_Issues.zip

[Gleeok]

Can you take a fresh working version and add in a single simple write and read to get the hang of binary file IO?

For example: Write out an array of 10000 values "[] = { 0,1,2,3,4,5,6,7,8,9, ... }" , then read it back in and print it out. If you don't feel like using a debugger properly then you simply /have/ to generate some output that can help you find out what you are doing wrong.

[ZoriaRPG]

Can you take a fresh working version and add in a single simple write and read to get the hang of binary file IO?

For example: Write out an array of 10000 values "[] = { 0,1,2,3,4,5,6,7,8,9, ... }" , then read it back in and print it out. If you don't feel like using a debugger properly then you simply /have/ to generate some output that can help you find out what you are doing wrong.

I may have found my answer without changing to VS... I'm not sync'd with the tables n defdata.cpp. It felt as if there was a third element to this, and there surely is. Thus might account for values shifting like this; eh? From what I can tell, the new data insertion is offsetting te vales by 10, resulting in reading the defaults from the wrong locations.

What would be the right way to insert the new array into this table?
I suppose I could just revert to a larger size for Defense instead now, though.



// flags flags2 tile width, height s_tile s_width s_height etile e_width e_height hp family cset anim e_anim, frate e_frate dp wdp weapon rate hrate step homing grumble item_set misc1 misc2 misc3 misc4 m5 m6 m7 m8 m9 m10 m11 m12 m13 m14 m15 bgsfx bosspal extend
{ 0x00000001, 0x00000000, 85, 1, 0, 0, 0, 0, 85, 1, 1, 2, eeGUY, 8, aNONE, aNONE, 0, 0, 1, 1, wNone, 0, 0, 0, 0, 0, isNONE, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, -1, -1, 0, { 0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0 }, 0, WAV_EDEAD, },


Didn't change much. Until I set up the other laptop, this may need to wait, unless someone else on the team wants to muck with it.

Is there any other file in which this type of thing is set? Perhaps somewhere in the quest header definitions?

[Gleeok]

The tables have nothing to do with corrupted data though.

Have a look at some specs for the c language aggregate initialization rules: https://msdn.microsoft.com/en-us/library/81k8cwsz.aspx

The scriptdefense array at the end will be zero initialized.


[edit] Also, I have to ask it, (sorry if it seems obvious to you, just trying to help) not trying to sound like a ass or anything: Without properly reading the version correctly you'll never be able to read/write a quest file without it becoming corrupted. This is because without it you introduce a "chicken and egg" logic bug regardless. The only way around this is to compile two versions, one for saving and one for loading.

[ZoriaRPG]

The tables have nothing to do with corrupted data though.

Have a look at some specs for the c language aggregate initialization rules: https://msdn.microsoft.com/en-us/library/81k8cwsz.aspx

The scriptdefense array at the end will be zero initialized.

I was lookin at comments like this, and I wasn't sure if that dat table needed me to reserve the space; as this is the case for many entries, even with zero'd fields.


bool reset_guys()
{
// The .dat file's guys definitions are always synchronised with defdata.cpp's - even the tile settings.
init_guys(V_GUYS);
return true;
}

Things like this tend to make me consider that proper synchronisation is mandatory at all times, and that the fields are explicitly prepared.

[edit] Also, I have to ask it, (sorry if it seems obvious to you, just trying to help) not trying to sound like a ass or anything: Without properly reading the version correctly you'll never be able to read/write a quest file without it becoming corrupted. This is because without it you introduce a "chicken and egg" logic bug regardless. The only way around this is to compile two versions, one for saving and one for loading.

No, that's something that I need to know. Could you expand on that, and specify: By version, do you mean the defined ZC version, V_GUYS, or something else? Have I done this somewhere, that you have observed?

Tell me this though: Why is expanding the enum with defLAST alone enough to corrupt this, when everything that thie packfile and editor generation code does relies on s size read fro this entry's placement?

If I add defs to the enum before edefLAST, and compile, with no other changes, the corruption still occurs. Is there anything with an explicit size that I missed? i simply fail to believe that the code can't resize itself around this, unless it's because it is shifting the values and the code isn't adaptive enough to comply. Even so, if it shifts the values, I'd expect it to mismatch when reading another quest, not the quest in memory, which is what led me to believe that the default values from defdata were involved.

Also, you said that the latest guyversion is 24, but in zdefs, it was 24 without my intervention.

[Gleeok]

Sure. You add an extra 10 byte array and read an older quest-> guy 0 gets loaded fine, guy 1 gets offset 10 bytes and is garbage, guy 2 gets offset 20 bytes and is garbage, guy 3 gets offset 30 bytes and is garbage. The old quest file here is the chicken, and the new format is the egg. You have to be able to load the chicken into the egg here. Without knowing that the quest file is a chicken you're just shoving an egg up the chicken's ass and turning a chicken into an egg by unnatural means, and then hilarity ensues as you have a messed up looking chicken that's bleeding all over your couch screaming in pain. You then beat it to death with a bat and try to figure out what's wrong with the chicken, but you can't. Make sense? :)



Moral of the story: Don't @#$% your chickens before they hatch. :P

[War Lord]

Sure. You add an extra 10 byte array and read an older quest-> guy 0 gets loaded fine, guy 1 gets offset 10 bytes and is garbage, guy 2 gets offset 20 bytes and is garbage, guy 3 gets offset 30 bytes and is garbage. The old quest file here is the chicken, and the new format is the egg. You have to be able to load the chicken into the egg here. Without knowing that the quest file is a chicken you're just shoving an egg up the chicken's ass and turning a chicken into an egg by unnatural means, and then hilarity ensues as you have a messed up looking chicken that's bleeding all over your couch screaming in pain. You then beat it to death with a bat and try to figure out what's wrong with the chicken, but you can't. Make sense? :)



Moral of the story: Don't @#$% your chickens before they hatch. :P


Fantastic Analogy
Well played

[ZoriaRPG]

Sure. You add an extra 10 byte array and read an older quest-> guy 0 gets loaded fine, guy 1 gets offset 10 bytes and is garbage, guy 2 gets offset 20 bytes and is garbage, guy 3 gets offset 30 bytes and is garbage. The old quest file here is the chicken, and the new format is the egg. You have to be able to load the chicken into the egg here. Without knowing that the quest file is a chicken you're just shoving an egg up the chicken's ass and turning a chicken into an egg by unnatural means, and then hilarity ensues as you have a messed up looking chicken that's bleeding all over your couch screaming in pain. You then beat it to death with a bat and try to figure out what's wrong with the chicken, but you can't. Make sense? :)

Oh, I know what's causing the offset. That's obvious. What I don't comprehend, is why when the loader reads the size of the defense array from the same place as the saver, why this occurs. Do I need to define a new guysversion for both sides, and load the new stuff only if guysversion is > N; because the editor panel doesn't do that at all for anything.

If I can't sort this, is it something you'd be willing to fix so that I have a basis of comparison on what you did?



Moral of the story: Don't @#$% your chickens before they hatch. :P

[DarkDragon]

Oh, sweet dear little baby infant Jesus.....

Please, please, pleeeease, be extremely careful when changing packfile formats. Even I go into "serious mode" when doing anything with that file.

This exactly.

There is unfortunately nothing "automatic" about the quest file format. The procedure for adding a field to the quest file is

1. Increment the appropriate section version number
2. Write the save code. Ensure you use the right packfile write command (bit length, endianness, etc) for the type of data you're trying to save.
3. In the quest loading code (in qst.cpp) in the place where you want to read in the new data, add a check for the section version number
- if it's less than the new version number, do NOT attempt to read the new data, but set the values for the new field to some reasonable default instead;
-- IMPORTANT: you might need to do this in several places in the code. Here is some fairly typical code structure:


if(s_version <= 2.0)
{
// some code for loading legacy 1.90 quests
}
else
{
// a **huge** block of code for loading modern quests

if(s_version <= 6.0)
{
// some code for handling 2.10 quests
}
else
{
// another huge block

// here is where you want to read in your new field
}
}

You need to make sure you cover *all possible* code paths when reading in the quest, and set default values correctly for all cases. In the example above, you will have to add code to set defaults in at least three places. There have been dozens of bugs in the past due to the old quest loading code not being updated properly as new fields are added.
- otherwise, read in the data from the file (using a matching command to the write command, obviously).

Then you test the shit out of the code, on both old quests and newly-created ones. The best-case scenario for a bug here is massive corruption when you try to load old quests. The worst case is a subtle bug that corrupts a quest only when it is loaded and then re-saved in ZQuest, totally screwing any quest author who encounters the bug and doesn't have regular backups of their WIP (yes, we tell them not to work on major projects using betas or RCs, but...)

[ZoriaRPG]

Sure. You add an extra 10 byte array and read an older quest-> guy 0 gets loaded fine, guy 1 gets offset 10 bytes and is garbage, guy 2 gets offset 20 bytes and is garbage, guy 3 gets offset 30 bytes and is garbage. The old quest file here is the chicken, and the new format is the egg. You have to be able to load the chicken into the egg here. Without knowing that the quest file is a chicken you're just shoving an egg up the chicken's ass and turning a chicken into an egg by unnatural means, and then hilarity ensues as you have a messed up looking chicken that's bleeding all over your couch screaming in pain. You then beat it to death with a bat and try to figure out what's wrong with the chicken, but you can't. Make sense? :)


Oh, I know what's causing the offset. That's obvious. What I don't comprehend, is why when the loader reads the size of the defense array from the same place as the saver, why this occurs. Do I need to define a new guysversion for both sides, and load the new stuff only if guysversion is > N; because the editor panel doesn't do that at all for anything.


I fixed it, anyway; so now everything works and I need to verify that older quests work properly, next.

For any-one who might be curious, this is what I did in qst.cpp:



if(guyversion > 24) // Add new guyversion conditional statement
{
for(int j=0; j<scriptDEFLAST; j++)
{
if(!p_getc(&(tempguy.scriptdefense[j]),f,keepdata))
{
return qe_invalid;
}
}
}
//Forward the value from old quests to the new attributes.
if(guyversion <= 24) // Offset the values intentionally?
{
for(int j=0; j<scriptDEFLAST; j++)
{
tempguy.scriptdefense[j] = tempguy.defense[edefSCRIPT] ;
}
}