Jump to content

Photo

Error codes for optimization of std.zh


  • Please log in to reply
4 replies to this topic

#1 Russ

Russ

    Caelan, the Encouraging

  • Administrators
  • Location:Washington

Posted 20 November 2020 - 03:40 AM

To provide some background, as I'm sure the dev team is well aware at this point, Yuurand's global is constantly brushing up against the ZASM line count limit. Today, we ran into an interesting issue where Moosh was getting global crashes when compiling, but I wasn't. We eventually traced this down to std.zh. I'm on a slightly older build of ZC with an std.zh version behind the current. Updating to the new one caused the global crash. I was curious and dug a little deeper to see what was changed and why this would've suddenly pushed us over. The offending functions here are the Get/SetLayerComboX functions. I've pasted here GetLayerComboD from 2.5.2 (for reference), 2.53.1 from earlier this year, and current 2.53.1.

2.5.2:

int GetLayerComboD(int layer, int pos) {
  if (layer==0)
    return Screen->ComboD[pos];
  else
    return Game->GetComboData(Screen->LayerMap(layer), Screen->LayerScreen(layer), pos);
}

Earlier this year:

int GetLayerComboD(int layer, int pos) 
{
	if (layer < 0 )
	{
		int err[]="Invalid layer passed to GetLayerComboD";
		TraceError(err,layer);
		return -1;
	}
	if (layer > 6 )
	{
		int err[]="Invalid layer passed to GetLayerComboD";
		TraceError(err,layer);
		return -1;
	}
	if (!layer) //layer 0
	{
		if ( !SETLAYERCOMBO_ALWAYS_USE_SETCOMBO )
		{
			return Screen->ComboD[pos];
		}
		else return Game->GetComboData(Game->GetCurMap(), Game->GetCurScreen(), pos);
	}
	
	return Game->GetComboData(Screen->LayerMap(layer), Screen->LayerScreen(layer), pos);
}

Current:

int GetLayerComboD(int layer, int pos) 
{
	if (layer < 0 )
	{
		int err[]="Invalid layer passed to GetLayerComboD";
		TraceError(err,layer);
		return -1;
	}
	if (layer > 6 )
	{
		int err[]="Invalid layer passed to GetLayerComboD";
		TraceError(err,layer);
		return -1;
	}
	if (!layer) //layer 0
	{
		if ( !SETLAYERCOMBO_ALWAYS_USE_SETCOMBO )
		{
			return Screen->ComboD[pos];
		}
		else return Game->GetComboData(Game->GetCurMap(), Game->GetCurScreen(), pos);
	}
	else
	{
		if ( Screen->LayerMap(layer) != -1 && Screen->LayerScreen(layer) != -1 )
		{
			return Game->GetComboData(Screen->LayerMap(layer), Screen->LayerScreen(layer), pos);
		}
		else
		{
			int err[]="The layer (%d) used by GetLayerComboD for the current screen are not properly configured.\n";
			printf(err, layer);
			return -1;
		}
	}
	
}

On the one hand, the function has several sanity checks added (one of which happened as a result of a bug on our end I believe), which is good. On the other hand, declaring three separate strings within the function makes it quite a bit heavier. These are useful for debugging, but increase the ZASM count of the functions, and therefore the count of any script that makes use of them, by an unacceptable amount.

What I propose is using standardized error codes instead. A single string could be declared in std.zh, such as

int err[] = "Error. Please consult the documentation and look for the following error code:\n";

The function could then be reduced to something like this:
 

int GetLayerComboD(int layer, int pos) {
	if (layer < 0 || layer > 6){
		printf(err);
		Trace(100);
		return -1;
	}
	if (!layer) //layer 0{
		if ( !SETLAYERCOMBO_ALWAYS_USE_SETCOMBO ){
			return Screen->ComboD[pos];
		}
		else return Game->GetComboData(Game->GetCurMap(), Game->GetCurScreen(), pos);
	}
	else{
		if ( Screen->LayerMap(layer) != -1 && Screen->LayerScreen(layer) != -1 ){
			return Game->GetComboData(Screen->LayerMap(layer), Screen->LayerScreen(layer), pos);
		}
		else{
			printf(err);
			Trace(101);
			return -1;
		}
	}
	
}

This still retains its usefulness in debugging errors while being significantly more lightweight.


  • Mani Kanina likes this

#2 Twilight Knight

Twilight Knight

    Tell all with glee, Argon's on PureZC

  • Members
  • Real Name:Sven
  • Location:Rotterdam, NL

Posted 20 November 2020 - 03:56 AM

Is this not a "delay of execution" type of fix for your problem?

 

Sure this will make that codebase smaller for now, probably significantly smaller since there are many error messages across many different functions, but I'm wondering how many new features and fixes it will take before you run into the same problem again.

 

 

Also, I personally like that the error is displayed directly in the console. On the other hand I wouldn't mind having to look them up by error code either if it is a proper solution for your problem.



#3 Russ

Russ

    Caelan, the Encouraging

  • Administrators
  • Location:Washington

Posted 20 November 2020 - 05:12 AM

Oh I'm thinking more for convenience of others here. I know I'm constantly dancing on the edge with my project's scope here, and I'm not above making my own bastardized copy of std.zh to eek every line of free ZASM space out of the global I can for my own personal use. I'm thinking more for others who are less experienced with ZC's quirks and run into problems. The ZASM line count limit is precariously small in 2.53, and it's pretty easy to run into by mistake. All these error messages, while convenient, make it much easier to do that.


  • Twilight Knight and Mani Kanina like this

#4 Emily

Emily

    Scripter / Dev

  • ZC Developers

Posted 20 November 2020 - 12:50 PM

I can see the use for 2.53. Though, for anyone running into the linecount error, my biggest recommendation is simply to update to 2.55 where that issue has been fixed for over 2 years.



#5 Timelord

Timelord

    The Timelord

  • Banned
  • Location:Prydon Academy

Posted 20 November 2020 - 07:22 PM

To provide some background, as I'm sure the dev team is well aware at this point, Yuurand's global is constantly brushing up against the ZASM line count limit. Today, we ran into an interesting issue where Moosh was getting global crashes when compiling, but I wasn't. We eventually traced this down to std.zh. I'm on a slightly older build of ZC with an std.zh version behind the current. Updating to the new one caused the global crash. I was curious and dug a little deeper to see what was changed and why this would've suddenly pushed us over. The offending functions here are the Get/SetLayerComboX functions. I've pasted here GetLayerComboD from 2.5.2 (for reference), 2.53.1 from earlier this year, and current 2.53.1.

2.5.2:

int GetLayerComboD(int layer, int pos) {
  if (layer==0)
    return Screen->ComboD[pos];
  else
    return Game->GetComboData(Screen->LayerMap(layer), Screen->LayerScreen(layer), pos);
}

Earlier this year:

int GetLayerComboD(int layer, int pos) 
{
	if (layer < 0 )
	{
		int err[]="Invalid layer passed to GetLayerComboD";
		TraceError(err,layer);
		return -1;
	}
	if (layer > 6 )
	{
		int err[]="Invalid layer passed to GetLayerComboD";
		TraceError(err,layer);
		return -1;
	}
	if (!layer) //layer 0
	{
		if ( !SETLAYERCOMBO_ALWAYS_USE_SETCOMBO )
		{
			return Screen->ComboD[pos];
		}
		else return Game->GetComboData(Game->GetCurMap(), Game->GetCurScreen(), pos);
	}
	
	return Game->GetComboData(Screen->LayerMap(layer), Screen->LayerScreen(layer), pos);
}

Current:

int GetLayerComboD(int layer, int pos) 
{
	if (layer < 0 )
	{
		int err[]="Invalid layer passed to GetLayerComboD";
		TraceError(err,layer);
		return -1;
	}
	if (layer > 6 )
	{
		int err[]="Invalid layer passed to GetLayerComboD";
		TraceError(err,layer);
		return -1;
	}
	if (!layer) //layer 0
	{
		if ( !SETLAYERCOMBO_ALWAYS_USE_SETCOMBO )
		{
			return Screen->ComboD[pos];
		}
		else return Game->GetComboData(Game->GetCurMap(), Game->GetCurScreen(), pos);
	}
	else
	{
		if ( Screen->LayerMap(layer) != -1 && Screen->LayerScreen(layer) != -1 )
		{
			return Game->GetComboData(Screen->LayerMap(layer), Screen->LayerScreen(layer), pos);
		}
		else
		{
			int err[]="The layer (%d) used by GetLayerComboD for the current screen are not properly configured.\n";
			printf(err, layer);
			return -1;
		}
	}
	
}

On the one hand, the function has several sanity checks added (one of which happened as a result of a bug on our end I believe), which is good. On the other hand, declaring three separate strings within the function makes it quite a bit heavier. These are useful for debugging, but increase the ZASM count of the functions, and therefore the count of any script that makes use of them, by an unacceptable amount.

What I propose is using standardized error codes instead. A single string could be declared in std.zh, such as

int err[] = "Error. Please consult the documentation and look for the following error code:\n";

The function could then be reduced to something like this:
 

int GetLayerComboD(int layer, int pos) {
	if (layer < 0 || layer > 6){
		printf(err);
		Trace(100);
		return -1;
	}
	if (!layer) //layer 0{
		if ( !SETLAYERCOMBO_ALWAYS_USE_SETCOMBO ){
			return Screen->ComboD[pos];
		}
		else return Game->GetComboData(Game->GetCurMap(), Game->GetCurScreen(), pos);
	}
	else{
		if ( Screen->LayerMap(layer) != -1 && Screen->LayerScreen(layer) != -1 ){
			return Game->GetComboData(Screen->LayerMap(layer), Screen->LayerScreen(layer), pos);
		}
		else{
			printf(err);
			Trace(101);
			return -1;
		}
	}
	
}

This still retains its usefulness in debugging errors while being significantly more lightweight.

 

 

In 2.53.x, this is only more lightweight in regards to ZASM lines, it is actually heavier and slower than the function in <std>. You can of course make a custom function and do a mass rename if you run into this. I add these error messages and sanity checks to <std> when users report bugs that end up being caused by misuse where the function misused did not produce an error that makes sense (or in some cases, none at all).

 

I will say that needing to look up numbered error codes is not a user convenience, particularly when running a game with a live debugging console where this type of metadata and logging is presented as the execution error occurs; and that type of error message would not present the values of the offending arguments. 

 

Compilation errors use numeric error codes, and these are documented. How many can you name?

 

For YR, I'd advice making a new function set of YR_GetLayerCombo* and YR_SetLayerCombo* and then mass replace all calls to these functions, if you want a limited error message. 

 

That said, I might work on some kind of model for standardised error codes, but they'd end up as something like this:

 

// Argument errors. 

1000 - Invalid value passed as argument.

1010 - Parameter passed was not an array, but array expected. 

1020- Expecting string and received non-string.

 

This would not take place of the verbose logging, but might be useful to have as part of it, but simply constructing a logical and an appropriate code list would take months

 

P.S. IDK if I have since revised the functions, but nothing should be calling PrintError(). It should all use printf().  If <std 1.5xx> is still using that for logging, then I need to fix that. I am pretty sure that <std 1.7xx> (for 2.55) no longer calls anything other than printf().




0 user(s) are reading this topic

0 members, 0 guests, 0 anonymous users