Jump to content

Photo

Copying Link->Item[i] into Array -- How to Crash ZC HARD!

bug 2.50.0

  • Please log in to reply
12 replies to this topic

#1 Timelord

Timelord

    The Timelord

  • Banned
  • Location:Prydon Academy

Posted 09 May 2015 - 08:07 AM

Here's one for the books...

If you want to discover how to cause ZC to crash, die, and burn... When I say crash, I mean, fully crash. Not freeze, but force close.

When trying to copy the index of Link->Item, for whatever reason, ZC will not permit a normal approach.

 

Both of the following functions cause ZC 2.50.0 to totally lock up, and terminate:

 

//Note: Equipment_BAK is an array initialised as follows:
// bool Equipment_BAK[256]={false};

void BackupEquipment(){
	for ( int i = 0; 1 < 255; i++) {
		if ( Link->Item[i] == true ) {
			Equipment_BAK[i] = true;
		}
	}
}

void BackupEquipment(){
	bool playerHas;
	for ( int i = 0; 1 < 255; i++) {
		if ( Link->Item[i] == true ) {
			Equipment_BAK[i] = true;
		}
	}
}

To circumvent ZC being sent to Bedlam, this is how I had to perform the operation:
 

bool CheckEquipment(int equip){
	for ( int i = 1; i < 255; i++){
		if ( Link->Item[i] == true ){
			return true;
		}
	}
}

void BackupEquipment(){
	for (int i = 1; i < 255; i++){
		Equipment_BAK[i] = CheckEquipment(i);
	}
}

For whatever reason, that works.

 

It's essentially the same bloody thing, save that in using two functions, one to read the Link->Item index, and return a value, and another to receive the value and change array elements, instead of doing it in one optimised function.

 

I've no idea why this is a problem for ZC, but Saffith, Gleeok, you might want to take note of it; and anyone else wanting to be able to copy the entire Link->Item array to a user-defined array will need to know about this.

 

I'm pretty sure that the other namespace arrays don't have this particular problem, but I expect I'll learn the hard way.

 

P.S. My guess is that Link->Item[x] may not have all of its values assigned somehow. It's a boolean though, so I can't imagine what's happening.


Edited by ZoriaRPG, 09 May 2015 - 08:10 AM.


#2 Saffith

Saffith

    IPv7 user

  • Members

Posted 09 May 2015 - 09:14 AM

This is the problem.
for ( int i = 0; 1 < 255; i++) {
It is true that Link->Item indices aren't validated as they ought to be, but that would still hang the game.

#3 justin

justin

    Adept

  • Members

Posted 09 May 2015 - 09:17 AM

In your first code you have two functions with the same name that the compiler can't tell apart. Also you have the comparison in the for loop 1 < 255 instead of i < 255 (The 255 should be 256 by the way).
what's the reason for the declared but unused playerHas variable?

Your second set of code is just as bad but without compiler breaking errors at least. First you send a parameter to the CheckEquipment function but don't actually use it. Instead you just cycle through Link->Item until you find one that is true which is not necessarily the one you are looking for. You also start counting your for loop from 1 instead of 0, and you should still be looking to be less than 256 not 255. Basically this code is going to set all of your backup array to true except for the first and last positions.

#4 Moosh

Moosh

    Tiny Little Questmaker

  • ZC Developers

Posted 09 May 2015 - 09:28 AM

As I learned fairly recently with my NPC script, referencing an invalid item (less than 0 or possibly more than 255 too) can cause ZC to crash. Because your for loop in the original function is set up in such a way that it would run forever I'd bet i is going above the max item ID and that's causing the crash. It might be a good idea to write new functions for reading and writing to Link's items that check if it's a valid item number first if you're the type to make careless mistakes.



#5 vaualbus

vaualbus

    Junior

  • Members
  • Real Name:Alberto
  • Location:Turin, Italy

Posted 09 May 2015 - 10:00 AM

Though I still I think that the engine have to make some check and eventually not crash if the script due stupid error.



#6 Alucard648

Alucard648

    Wizard

  • Members
  • Location:castle Dracula

Posted 09 May 2015 - 01:50 PM

The array is not initialized properly. You need to initialize every element of array, like here:

bool BackItems[256];
for( int i = 0; i<256; i++){
BackItems[i] = false;
}

EDIT:

for ( int i = 0; 1 < 255; i++) 

Looks like a stupid error...


Edited by Alucard648, 09 May 2015 - 01:52 PM.


#7 justin

justin

    Adept

  • Members

Posted 09 May 2015 - 07:46 PM

I think a boolean array gets initialized as false anyways but ya its probably a wise idea to have a function that wipes and initializes arrays. I think Zoria thought that putting {false} would initialize the whole array to false but really I think that only would do the first array position.

void ArrayInit(bool anArray)
{
 int size = SizeOfArray(anArray);
 if (size < 2) return;
 for (int i = 0; i < size; i++)
 {
  anArray[i] = false;
 }
}


#8 Timelord

Timelord

    The Timelord

  • Banned
  • Location:Prydon Academy

Posted 09 May 2015 - 11:12 PM

This is the problem.

for ( int i = 0; 1 < 255; i++) {
It is true that Link->Item indices aren't validated as they ought to be, but that would still hang the game.

Oops. I'm shocked that I didn't see that. Managed a numeral 1 instead of an i there. Infinite for loop, huzzah.

So right, looks like it was my fault.

Now I need to go back and see if fixing that has any other issues, but what I've done works, and gives me a second utility function to use for other purposes.

Edited by ZoriaRPG, 10 May 2015 - 12:14 AM.


#9 Timelord

Timelord

    The Timelord

  • Banned
  • Location:Prydon Academy

Posted 10 May 2015 - 03:43 AM

This is the problem.

for ( int i = 0; 1 < 255; i++) {
It is true that Link->Item indices aren't validated as they ought to be, but that would still hang the game.

 

Oops. I'm shocked that I didn't see that. Managed a numeral 1 instead of an i there. Infinite for loop, huzzah.

So right, looks like it was my fault.

Now I need to go back and see if fixing that has any other issues, but what I've done works, and gives me a second utility function to use for other purposes.


---

I aced it out: I had another typo set there, with > instead of < in a few spots. That's the result of typing with a cat sitting next to the laptop, on my chest.

I was still shocked that ZC died, entirely. Usually any infinite loop would cause it to freeze, not terminate; but I suppose there are exceptions.

So, right. If I ever see ZC do that again, I'll look for a random numeral where a variable belongs.

---

The final functions are as follows:
 

void RestoreEquipment(){
	//int state;
	for ( int i = 0; i < 255; i++ ) {
		if ( Link->Item[i] == true && Equipment_BAK[i] == false ) {
			Link->Item[i] = false;
		}
		else if ( Link->Item[i] == false && Equipment_BAK[i] == true ) {
			Link->Item[i] = true;
		}
		//Waitframe();
	}
}

void BackupEquipment(){
	//int state;
	for ( int i = 0; i < 255; i++ ) {
		if ( Link->Item[i] == true && Equipment_BAK[i] == false ) {
			Equipment_BAK[i] = true;
		}
		else if ( Link->Item[i] == false && Equipment_BAK[i] == true ) {
			Equipment_BAK[i] = false;
		}
		//Waitframe();
	}
}

I likely don't need those if / else statements, but ZC can become angry when modifying Link->Item if you don't use them, so they're required in the restore function. I left them in the saving function for my own peace of mind.

As to why there is a commented out 'state' var there, it's for future use with BackupEquipment(int state). Just a reminder to do that, for future multiple states.

 

I think a boolean array gets initialized as false anyways but ya its probably a wise idea to have a function that wipes and initializes arrays. I think Zoria thought that putting {false} would initialize the whole array to false but really I think that only would do the first array position.

The array is not initialized properly. You need to initialize every element of array, like here:

bool BackItems[256];
for( int i = 0; i<256; i++){
BackItems[i] = false;
}

Not true... You can't declare an array with a specified index size in ZC, without declaring at least one index value. The way that I do it, is the way that ZC expects me to do it, and that's why I have stuff like:

 

I stand corrected.

 

---

If you want an array with values other than the default, you'd need to wipe it with a for loop. Thus, if my default value is 'true', I'd need to do that, which is why I code boolean arrays around false being their base.

The same applies to float, or int arrays. If I needed them to initialise with each element !=0 , then I'd need to perform an array-wide change like that, but I try to code myt functions around 0 as a default. There are only a few places where '1' as a default is required, such as searching some ZC (internal) arrays, and for those, it'd be handy.

That of course, primarily applies to nesting arrays into functions. I'll probably need to do that when I write the backup tables for Screen->NPCs, and all of their associated arrays (NPC_Weapon, etc.) and to track existing EWeapons, and such; but things like FFCs, and ghosted enemies won't obey these types of commands, unless I hardcode them to report all of their vars to other arrays, and that mates, will become messy.

The best solution I have there, is to track the position of FFCs, and hope for the best, because trying to savestate exactly what a ghosted enemy is doing would be painful, and this isn't meant for that.

I might do that for my own scripted enemies, and bosses, to make playtesting/bugtesting them easier, but I'd need to set up arrays for BossPhase, and such, and have them report to that array, so that the game can restore to the player, while the ghosted enemy is doing a function that I need to debug.

 

[ ...]

bool checks[4096]={false};

...does work. Index [0] is set to false manually, and indices [1] to [4095] are set to false by the compiler.

Other compilers permit you to declare an array of a fixed size without declaring any of its elements, but ZC/Buffalo does not.

The alternative, or manually entering false, or cipher elements, would be hideously tedious.

I don't think that Saffith or Gleeok could fix this, as it's a compiler quirk.
I am however curious if returning optimisation is useful in ZC with large arrays, and how large an array needs to be before it becomes important; e.g.:

 for (int i = 1; i < 200; ++i)

...versus...

 for (int i = 1; i < 200; i++)

The overall result is the same, but one may favour one over the other, and one of the two may provide more optimal performance. Not that it matters in general. I use i++ or i-- in my for loops most of the time.


Edited by ZoriaRPG, 10 May 2015 - 09:01 AM.


#10 justin

justin

    Adept

  • Members

Posted 10 May 2015 - 06:57 AM

you don't need to initialize anything.

 

bool anArray[10];  // will work to create the array and initialize to default values

 

By the way you made the same error 1 < 255 in your last code about optimization

 

edit:

and you still have the error of not getting all the positions of your array.  if your array size is 256 then you have positions 0-255.  if you say i < 255 (as you have) then you will never access position 255.  you want it to be i < 256 to access position 255.

 

i have two questions.  

1) why do you have a commented Waitframe(); inside your for loops?  

2) how does "ZC became angry when modifying Link->Item" without the if statements?  That makes no sense to me.  you are still setting every position of the array with the if statement so the if statement should be irrelevant in this case.  are you sure that when zc became angry you hadn't made some other mistake due to a cat in the lap?

 

void RestoreEquipment()
{
 //int state;
 for ( int i = 0; i < 256; i++ )
 {
  Link->Item[i] = Equipment_BAK[i];
 }
}

Edited by justin, 10 May 2015 - 09:07 AM.


#11 Timelord

Timelord

    The Timelord

  • Banned
  • Location:Prydon Academy

Posted 10 May 2015 - 09:09 AM

you don't need to initialize anything. Just bool anArray[10]; will work to create the array and initialize to default values.

By the way you made the same error 1 < 255 in your last code about optimization

 

It seems that I've been giving myself pointless extra work, as I recalled ZC requiring curly-braced array structures (excluding strings) at all times, possibly because I had to manually initialise so many of them and rarely needed empty arrays until I started doing absolutely mad things, like setting up duplication sets.

 

In either case, formatting the array when creating it is an unnecessary extra step, unless you want a special value. I suppose it may be useful in other applications, if the default value is something odd, but it should never be needed here.

 

For the most part, when when I had them empty, I manually initialised them so that I could form them into columns and rows as tables too.

 

I'll try not to allow that to fall through one of the holes in my head and do that in future work. The compiler couldn't care less if it's initialised using one element in the first position, or with none; and either will produce the same assembled code. I'm sort of curious what the ratio is, between lines of ZScript, and ZASM output. I know that I have around 325,000 assembly instructions in play, but I've yet to do a linecount of the source script files.

 

I only know how many assembly instructions exist, because I occasionally export the assembled code, and archive that, and I had to widen my margin to see the LC numerals.

 

The above error, I blame on copy/paste within the message body, but it might have been my doing. When typing 'int i = 1', my fingers tend to hover over both the 1, and the i; and I'm potentially prone to tapping the wrong key. After doing this for six to ten hours at a stretch, ... let's just say I become weary, and miss things.

 

The only reason that I started this thread, was because the other, many sets of backup arrays didn't cause ZC to crash: Only the items array gave me a hint of trouble, and coincidentally, it was the only one with an error in it. Because of how much hell Link->Item[] can cause in general, I came to the wrong conclusion, and didn't spend enough time staring at it.

 

All of the completed backup functions now work though, which is another step toward me end goal.


Edited by ZoriaRPG, 10 May 2015 - 09:20 AM.


#12 Gleeok

Gleeok

    It's dangerous to dough alone, bake this.

  • Members
  • Real Name:Pillsbury
  • Location:Magical Land of Dough

Posted 11 May 2015 - 01:19 AM

Coincidentally, I stopped using 'l' as a variable name long ago because I always read it as '1'.

I had a stack corruption bug that randomly happened that was extremely difficult to debug. Turns out it was a typo in a container function that just happened to work fine on heap memory, so it stayed hidden. Very dumb. ..So yeah, shit happens. Don't worry about it.

#13 Timelord

Timelord

    The Timelord

  • Banned
  • Location:Prydon Academy

Posted 11 May 2015 - 05:11 AM

Coincidentally, I stopped using 'l' as a variable name long ago because I always read it as '1'.

I had a stack corruption bug that randomly happened that was extremely difficult to debug. Turns out it was a typo in a container function that just happened to work fine on heap memory, so it stayed hidden. Very dumb. ..So yeah, shit happens. Don't worry about it.

 

'I' should probably do likewise. The longer we stare at these screens, the worse our vision degrades. Good anecdote, and prudent choice Gleeok. :)





Also tagged with one or more of these keywords: bug, 2.50.0

0 user(s) are reading this topic

0 members, 0 guests, 0 anonymous users