Fixed Crash Due to ClearCharacter Called on Mutinuous Captain

[Mere_Mortal]

In my fix... bSeaActive interferes with encounter generation, as in trying to use dead people. Best to not use that clause methinks.

Ship_Add2Sea ERROR: Please post your compile.log file at piratesahoy.net!
Trying to log in dead captain:

Ship_SetTaskMove ERROR: Please post your compile.log file at piratesahoy.net!
Trying to give task to dead captain:

 

[Pieter Boelen]

In my fix... bSeaActive interferes with encounter generation, as in trying to use dead people. Best to not use that clause methinks.

I'll try to come up with something better. Hopefully this weekend....

 

[Pieter Boelen]

The correct place to fix this is probably here in PROGRAM\Characters\officers.c:

int FindFreeRandomOfficer()
{
   // this function rewritten by KAM so new officers don't keep overwriting captains on shore leave [changes made by MAXIMUS 07.10.2007]

   for(int tempnum=0; tempnum<=NUM_RANDOM_OFFICERS; tempnum++) // NK 05-03-30 we now use char not exist to stop loop, so we can have any number of enc_off.
   {
     string tempid = "Enc_Officer_" + tempnum;
     int tempidx = GetCharacterIndex(tempid);
     if(tempidx==-1) break; // NK 05-03-30 ditto
     ref tempChar = GetCharacter(tempidx);

     if(bAllies(tempChar)) { continue; }
     if(tempChar.location == loadedLocation.id) { continue; } // PB: To prevent prospective officers from disappearing when you're talking to them

     ClearCharacter(tempChar); // PB: Completely erase the unused character

     return tempidx;
   }

   return -1;
}

It could also be FindFreeCabinCaptain() in PROGRAM\Dialog_func.c or LogoffCharactersFromLocation from PROGRAM\Characters\characters_login.c, but that seems a bit less likely to me.

Main question is: How do you distinguish between an "Enc_Officer_" character (e.g. a randomly generated officer) who is a mutineer and therefore still necessary
and the same character after you let him escape, you won't ever see him again and he can therefore be erased?
I've got to admit that at the moment I don't really have an answer to that question, so I'd welcome any clever ideas anyone might have....

 

[Mere_Mortal]

By the time the ransack menu appears, are the locations for the player and mutineer reset? If so then maybe they are no longer at the same place so that catch fails and the function proceeds to clear the officer. Does it actually matter if he’s not immediately cleared since this will take place at some point later on anyway, so eventually he would be removed if he is not recaptured? Besides, if he’s killed then is this function even necessary in such a case? Next time I have a mutiny I’ll drop the attributes for the captain to see what is the same/deleted/changed compared to when he was a companion - maybe that would allow for ideas to check the attributes, so as to add another exclusion to the function.

 

[Pieter Boelen]

Some Trace statements may be required to double-check exactly when the guy is cleared.
My suspicion is that he may get cleared when the boarding starts, which is too early.
At that point, the enemy captain is probably either at sea or in his cabin, but not in the same location as the player.

 

[Mere_Mortal]

I guess I’ll look at the save game again because this here...

XP ERROR: Character was missing rank during post init
index = 2001
id =
location = none
locator =
group =
experience = 1
perks =
freepoints = 1
skill =
freeskill = 0
quest =
officertype = civilian
officerprice = 0
rank = 1
money = 33
loyality = 10
alignment = bad
homelocation = none
group =
locator =
homestate = citizen
name =
lastname =

...suggests he has no model, face, weapon, skills, etc. That was not the case during the cabin fight, but then again that log dump could have been triggered by one of his officers rather than himself. I think this also appeared as soon as the ransack menu came up so it may not entirely explain anything in itself. I’ll look at the save now actually.

 

[Pieter Boelen]

After the character has been cleared, indeed he'd have lost almost all attributes.

May be time to return the additional Trace lines I suggested here:
Fixed - Empty officer appearing in Passengers | PiratesAhoy!

 

[Mere_Mortal]

I have a bunch of dump logs now.

1. Map, before mutiny
2. Sea, mutiny started
3. Boarding, first deck
   
4. Boarding, cabin
5. Ransack menu

The second dump doesn’t change much from the first. We pretty much only see these of interest...

seaai =
  group =
    name = Mutineers

flags =
  pirate = 2
  texture = 3
  dorefresh = 1

mutiny_note = 1
mutineer = 1

The third has some additional changes, which look rather interesting...

chr_ai =
  group = brdenemy

dialog =
  filename = Cabinfight_dialog.c
  currentnode = First time

location = Boarding_Cabin_medium

status = dead
position = captive

Dead and captive, eh? Then there’s the cabin fight...

chr_ai =
  group = corpses

corpse = 1

[no dialogue]

So he was officially dead even though I was about to fight him. This doesn’t add up.

Obviously the fifth log saw a wipe of his attributes, which is fine because he was dead.

Now, when I did this I removed the officers from his ship beforehand for ease of debugging, and it did not crash after sinking the ship (which was again a Lateen Caravel by this point). Therefore, the crash could be more to do with the officers than the captain. In which case, I guess we have two problems here.

 

[Pieter Boelen]

I have absolutely NO clue if this is going to work, but it might be worth a try....

int FindFreeRandomOfficer()
{
   // this function rewritten by KAM so new officers don't keep overwriting captains on shore leave [changes made by MAXIMUS 07.10.2007]

   for(int tempnum=0; tempnum<=NUM_RANDOM_OFFICERS; tempnum++) // NK 05-03-30 we now use char not exist to stop loop, so we can have any number of enc_off.
   {
     string tempid = "Enc_Officer_" + tempnum;
     int tempidx = GetCharacterIndex(tempid);
     if(tempidx==-1) break; // NK 05-03-30 ditto
     ref tempChar = GetCharacter(tempidx);

     if(bAllies(tempChar)) { continue; }
     if(tempChar.location == loadedLocation.id) { continue; }     // PB: To prevent prospective officers from disappearing when you're talking to them
     if(LAi_IsBoardingProcess() && tempChar.id == boarding_enemy.id)   // PB: Skip the captain of the ship you're currently boarding

     ClearCharacter(tempChar); // PB: Completely erase the unused character

     return tempidx;
   }

   return -1;
}

And:

int FindFreeCabinCaptain()
{
   for(int tempnum=0; tempnum<=CABINCAPTAINS_MAX; tempnum++)
   {
     string tempid = "Enc_CabinCaptain_" + tempnum;
     int tempidx = GetCharacterIndex(tempid);
     if(tempidx==-1) break;
     ref tempChar = GetCharacter(tempidx);

     if(bAllies(tempChar)) { continue; }
     if(tempChar.location == loadedLocation.id) { continue; }     // PB: To prevent prospective officers from disappearing when you're talking to them
     if(LAi_IsBoardingProcess() && tempChar.id == boarding_enemy.id)   // PB: Skip the captain of the ship you're currently boarding

     ClearCharacter(tempChar); // PB: Completely erase the unused character

     return tempidx;
   }

   return -1;
}

As far as I can tell, ''boarding_enemy" is a global reference to the captain of the ship you're currently boarding.
So if that is the character the game is considering to remove, better skip that one....

This probably won't solve any issues related to having officers on an NPC ship though.
That usually never happens, so I've got no clue how well the game would handle that.

 

[Mere_Mortal]

This probably won't solve any issues related to having officers on an NPC ship though.
That usually never happens, so I've got no clue how well the game would handle that.

Except during a mutiny. Thinking about that time I was talking about the possibility of allowing NPC ships to have officers, it’s nice to nip this one just in case it ever becomes a reality.

As for the two clauses you put there, what about instead using OR rather than AND? I mean, I reckon one will always be true if the other is. I wonder if doing it that way would actually go so far as to catch the officers as well?

Err, by the way... did you miss the continue in that code you added?

Maybe this?

if (bAllies(tempChar) || LAi_IsBoardingProcess() || tempChar.location == loadedLocation.id) || tempChar.id == boarding_enemy.id) continue;

 

[Pieter Boelen]

Having them all as OR would probably cause every character in the loop to be skipped,
which means that the 'FindFreeCabinCaptain()' function would fail to find empty character slot.

The 'tempChar.id == boarding_enemy.id' section is meant to skip only the captain of the enemy ship.
Since I don't know if that variable does anything sensible outside boardings (probably not),
I added that 'LAi_IsBoardingProcess()' check in front of it to avoid it doing weird things.

As you point out though, we don't want only the captain to be skipped, but also any officers of that captain.
Normally such an officer wouldn't have "Enc_CabinCaptain_" or "Enc_Officer_" as ID, but you never do know.
During player companion mutinies, the officers involved could indeed be anything.

The ideal way to handle that would be by using the 'IsOfficerOf' function.
Maybe something like:

int FindFreeCabinCaptain()
{
   for(int tempnum=0; tempnum<=CABINCAPTAINS_MAX; tempnum++)
   {
     string tempid = "Enc_CabinCaptain_" + tempnum;
     int tempidx = GetCharacterIndex(tempid);
     if(tempidx==-1) break;
     ref tempChar = GetCharacter(tempidx);

     if(bAllies(tempChar))                   { continue; }
     if(tempChar.location == loadedLocation.id)         { continue; }   // PB: To prevent prospective officers from disappearing when you're talking to them
     if(LAi_IsBoardingProcess())
     {
       if (tempChar.id == boarding_enemy.id)         { continue; }   // PB: Skip the captain of the ship you're currently boarding
       if (IsOfficerOf(boarding_enemy, tempChar))       { continue; }   // MM: Skip also any officers assigned to this ship
     }

     ClearCharacter(tempChar); // PB: Completely erase the unused character

     return tempidx;
   }

   return -1;
}

I am not 100% certain that will work as I haven't tested it at all.
The 'boarding_enemy' and 'tempChar' may also need reversing in that section.

Err, by the way... did you miss the continue in that code you added?

Yes, indeed I did. Good catch!

 

[Mere_Mortal]

I guess I’ll do what I did with the captain to see what changes are made to the officers during this process as well. I reckon the captain would be safe with that fix, assuming the variables are correct like you say... but who knows if the officers are even still assigned to him by the time the boarding takes place.

 

[Pieter Boelen]

but who knows if the officers are even still assigned to him by the time the boarding takes place.

I have absolutely no clue. Having officers on NPC ships is a brand new concept.
That'll require some figuring out....

 

[Mere_Mortal]

Seems there are no changes being made to the officers between the mutiny starting at the world map and the ransack menu (with the single exception of experience). Therefore, the changes to or indeed the removal of the captain, is probably breaking some important links. I think we might have seen an officer being removed in the logs, but if he ended up with no captain to work for then it might be fair enough that he was also removed. As such, I’m going to hazard a guess in saying that addressing the problem with the captain may well fix the entire problem.

 

[Pieter Boelen]

The 'bAllies' check prevents any characters in the player's service from being cleared. But any other characters are fair game.
Normally the idea was that characters with those particular IDs are meant specifically for the player party.

If NPC officers are given an alternate type of ID, they won't be cleared by this code.
That being said, if all NPC ships get officers, they do need cleaning up at some point or we'd get character overkill.
But anyway, we're not that far yet.

Seems there are no changes being made to the officers between the mutiny starting at the world map and the ransack menu (with the single exception of experience).

HA, so that part actually DOES work!
I did do a bit of rewriting there a while back, trying to do it in such a way that NPC officers might also work even if they hardly ever exist.

 

[Mere_Mortal]

No joy, bud. Looks like LAi_boarding.c is doing this as well using DeleteCharacter.

Actually, you know what? Seemingly my instincts were spot on in the first place...

Looks to me like the mutineer is being deleted.

Since it worked, might it be easier to do it as I did originally with ClearCharacter?

Also, I assume this is correct?

int FindFreeRandomOfficer()
{
   for(int tempnum=0; tempnum<=NUM_RANDOM_OFFICERS; tempnum++)
   {
     string tempid = "Enc_Officer_" + tempnum;
     int tempidx = GetCharacterIndex(tempid);
     if(tempidx==-1) break;
     ref tempChar = GetCharacter(tempidx);
     if(bAllies(tempChar))               { continue; }
     if(tempChar.location == loadedLocation.id)     { continue; }   // PB: To prevent prospective officers from disappearing when you're talking to them
     if(LAi_IsBoardingProcess())
     {
       if (tempChar.id == boarding_enemy.id)     { continue; }   // PB: Skip the captain of the ship you're currently boarding
       if (IsOfficerOf(boarding_enemy, tempChar))   { continue; }   // MM: Skip also any officers assigned to this ship
     }
     Trace("ClearCharacter called from FindFreeRandomOfficer");
     ClearCharacter(tempChar); // PB: Completely erase the unused character
     return tempidx;
   }
   return -1;
}

 

[Pieter Boelen]

Looks like LAi_boarding.c is doing this as well using DeleteCharacter.

DeleteCharacter just removes a character from the scene, I think.

ClearCharacter erases all that character's attributes. I added that a while back and it's been causing unexpected trouble once in a while.
But if we sort out those issues, I do think it is cleaner and safer to do that for character IDs that are continuously being reused.

Since it worked, might it be easier to do it as I did originally with ClearCharacter?

What exactly did you do there again?
If I recall, you aborted that function if sea is active or you're in a boarding, right?
I'd really rather not have to do that....

Also, I assume this is correct?

Looks correct to me.

No joy, bud.

In what way is it not working? The fact that ClearCharacter is being called is not actually a problem.
It is meant to be called. Just not on any characters who are still needed.
You'll need to enable the Trace line to indicate which character is being cleared.
EDIT: See here: Fixed - Empty officer appearing in Passengers | PiratesAhoy!

If the captain or an officer of the ship you're boarding is being cleared, that is not good.
But if any other random character who already served his purpose is being cleared, that is exactly what it is supposed to do.

Since the character IDs are being recycled, an "available slot" needs to be found.
That slot is then completely erased before a new character is placed into it.
Problems occur when the game thinks a slot is "available" when it really isn't.

 

[Mere_Mortal]

If I recall, you aborted that function if sea is active or you're in a boarding, right? I'd really rather not have to do that....

Fair enough. I did indeed abort, but bSeaActive should not be used as it would cause expired characters to be selected for encounters.

If the captain or an officer of the ship you're boarding is being cleared, that is not good.

Yes, this is what’s happening. That is why the ship becomes a Lateen Caravel, because the ship attributes are removed from the captain. If an officer is affected as well then the game will crash after leaving the ransack screen (oddly enough, I don’t think it’ll crash if it’s just the captain).

The code you did looks like it should work, but for whatever reason it’s having no effect.

 

[Pieter Boelen]

Only way to know for sure is to do some tracing and dumping to see what actually happens.
Does that if even get triggered? If so, what does a Dump on that enemy give?
Maybe that hasn't been correctly initialized yet?

 

[Pieter Boelen]

@Mere_Mortal: Did you by any chance get round to testing exactly what happens here yet?
Ideally I'd like to have this one tackled reasonably well before I post another update.
But the work is stacking up on my side (also professionally) and I need a bit of a breather....