Fixed CharacterFromID() crashes with invalid input

[Mere_Mortal]

This function is extremely liable to crashing the game if the input is not correct.

[QUESTS\quests.c]

ref CharacterFromID(string characterID)
{
   // PB: Error checking -->
   if(characterID == "") characterID = "Boat1234"; // just some random character to prevent error logs
   if(sti(GetCharacterIndex(characterID)) == -1)
   {
     TraceAndLog("ERROR - Missing Character ID: " + characterID);
   }
   // PB: Error checking <--
   return &characters[GetCharacterIndex(characterID)];
}

As such, I recommend including a return with the TraceAndLog in order to prevent passing invalid data.
I suppose there are two choices here: either return false or return &NullCharacter. I’m using the latter.

 

[Pieter Boelen]

Latter sounds good.

Strange though; I've never actually had it crash before.
I'd always get the error on the screen and game continues fine afterwards.

 

[Mere_Mortal]

Maybe in general a there isn’t a problem, it’s just that I’m occasionally chucking a non-existent ID at the function. It has been annoying me for a while now but thankfully this stops the crash. I guess the real fix is of course to not use dodgy input in the first place.

Using the null character is probably a good idea actually because there could still be problems down the line when it’s filtered back to where it’s supposed to go. I guess if Sir Richard Dutton (whoever he even is) ends up where he shouldn’t be then at least it’s probably safer.

 

[Pieter Boelen]

Maybe in general a there isn’t a problem, it’s just that I’m occasionally chucking a non-existent ID at the function. It has been annoying me for a while now but thankfully this stops the crash. I guess the real fix is of course to not use dodgy input in the first place.

Normally non-existent IDs don't happen very often, so even if it COULD cause a crash, it normally wouldn't because it doesn't happen.
So your suggestion definitely sounds like a good idea.

I guess if Sir Richard Dutton (whoever he even is) ends up where he shouldn’t be then at least it’s probably safer.

Richard Dutton? AGAIN??? For God's sake, I HATE THAT GUY!
He's basically a non-existent bit of randomness that totally shouldn't show up, but randomly does anyway whenever code isn't quite right.
It is bizarre and I have no clue how that is even possible. Fix him showing up in one spot and he'll show up somewhere else instead.

The only place he "exists" in the code is in Periods.c as he's a governor in one specific time period:
List of governors of Barbados - Wikipedia, the free encyclopedia
But that name should be used exclusively as a rename of an existing governor and ONLY in the period that he exists.

 

[Mere_Mortal]

He’s basically a place-holder. I noticed in that function you used “Boat1234” - I guess this guy pretty much serves the exact same purpose so that must be his boat, right? So yeah, it shouldn’t be possible but if so then at least there’s a character reference in place rather than a null value.

 

[Pieter Boelen]

Like this then I think?

ref CharacterFromID(string characterID)
{
   // PB: Error checking -->
   if(characterID == "") characterID = "Boat1234"; // just some random character to prevent error logs
   if(sti(GetCharacterIndex(characterID)) == -1)
   {
     TraceAndLog("ERROR - Missing Character ID: " + characterID);
     return &NullCharacter; // MM
   }
   // PB: Error checking <--
   return &characters[GetCharacterIndex(characterID)];
}

 

[Mere_Mortal]

Exactly, aye.

 

[Pieter Boelen]

Sounds like a plan!

 

[Pieter Boelen]

This now included in here: Mod Release - Build 14 Beta 4.1 WIP | PiratesAhoy!