Fixed Opium Smuggling: Errors With "List of Opium Buyers"

[Talisman]

Playing Build 14 Beta 4.1 (16 May)

I have some Opium - go to tavern to ask Smuggler there about it.

Ask for list of Buyers - smuggler says "here is list & book about Opium Smuggling "- dialog ends & game freezes .

( as it tries to give me list of buyers & Smuggling book and update Quest book)

The game freezes completely and "Locks Up" my computer - I end up having to restart my computer.

No error log generated

Save is in tavern about to talk to smuggler.

 

[Pieter Boelen]

Thanks for the report. I'll check it probably tomorrow.
I did change some stuff there; maybe I made a mistake.

 

[Mere_Mortal]

This is the problem right here, it’s basically looping through a bazillion characters...

[QUESTS\quests_side.c]

   case "Smugglers Opium Explain":
      pchar.quest.smuggling_guild.opium_explain = TRUE;
      int NumMembers = 3+rand(3);
      GetSmugglingGuildMembers(NumMembers, "OpiumBuyers", "OpiumBuyer.c");

[smuggling.c]

void GetSmugglingGuildMembers(int NumMembers, String Category, String Dialog)
{
   ...
   if(DEBUG_SMUGGLING>2)trace("SMUGGLING GUILD MEMBERS pass officer check");
   if(GetAttribute(chr, "abordagemode") != "1")
   {
      ...

I changed the debug value to see what would happen and my compile log was over a megabyte within seconds.

So it doesn’t hang per-se, it was quite merrily working away filling up the log and eventually it would probably have finished.

I end up having to restart my computer.

Ctrl+Shift+Escape (or Ctrl+Alt+Delete), press “P”, then “Delete” and hit “Enter” for good measure.

The attached file reinstates the previous code which itself seems to be wrong, but it doesn’t cause a hang.

 

[Pieter Boelen]

I thought it would be in that spot. Well, that definitely proves I messed it up.
Will see what is going on tomorrow.

 

[Mere_Mortal]

I just thought, since this was the original code...

if(!CheckAttribute(chr,"abordagemode"))
{
  if(chr.abordagemode == "1") board = true;
}

Could it be that somebody has put that “!” there to deliberately break the check because they knew it was causing a hang?

 

[Pieter Boelen]

That does indeed look quite bizarre!

I doubt it is intentional though. Should probably be the other way around.

 

[Mere_Mortal]

The other way around is precisely what causes the hang, that is why I think it was deliberate.

Forget that stupid check, it’s not the problem. This is...

if(DEBUG_SMUGGLING<3)trace("SMUGGLING GUILD MEMBERS pass officer check");
   //if(GetAttribute(chr, "abordagemode") != "1") // <-- latest code
   if(chr.abordagemode == "1")
   {
      ...

That fixes it, although it could break the intended purpose. If chr.abordagemode is set to zero then it will hang.

Also note that change to DEBUG_SMUGGLING. I’m currently watching compile.log, it’s at 15mb and counting.

Actually, I know what’s going on here. Because the check is failing, the chosen variable is not being incremented and is always zero, and therefore always less than NumMembers. So the while loop is recursive.

 

[Pieter Boelen]

Infinite loop? Yep, that's bad.

And that code is indeed something I changed not too long ago.
Clearly I didn't think it through properly; I'll do it better tomorrow, I promise.

 

[Mere_Mortal]

I’ve totally simplified the function and it seems to be having exactly the same effect...

void GetSmugglingGuildMembers(int NumMembers, string Category, string Dialog)
{
   int Charnum = GetCharacterIndex("Boat1234");
   int chosen = 0;
   int chridx = 0;
   ref pchar = GetMainCharacter();
   string index = "";

   while (chosen < NumMembers) {
     chridx = rand(Charnum);
     ref chr = GetCharacter(chridx);

     if (CharacterIsDead(chr)) continue;
     if (!CheckAttribute(chr,"chr_ai.type")) continue;
     if (CheckAttribute(chr,"questchar")) continue;
     if (CheckAttribute(chr,"quest.officertype")) continue;
     if (isSoldier(chr) == UNKNOWN_NATION) continue;
     if (GetAttribute(chr,"isOfficer") == true) continue;
     if (GetAttribute(chr,"location") == "none") continue;
     if (GetAttribute(chr,"location") == "") continue;
     if (GetAttribute(chr,"abordagemode") == false) continue;
     if (GetAttribute(chr,"chr_ai.type") == LAI_TYPE_MERCHANT) continue;
     if (GetAttribute(chr,"Dialog.Filename") == "governor.c") continue;
     if (GetAttribute(chr,"Dialog.Filename.GroupDialog") == "governor.c") continue;
     if (GetAttribute(chr,"Dialog.Filename") == "Smuggler_OnShore_dialog.c") continue;
     if (GetAttribute(chr,"Dialog.Filename") == "Agent_dialog.c") continue;

     if (!CheckAttribute(chr,"Dialog.Filename.SmugglingGuild")) {
       if (chr.lastname == "") {
         SetRandomNameToCharacter(chr);
       }
       index = Category + chosen;
       pchar.quest.smuggling_guild.people.(Category).(index) = chr.id;
       pchar.quest.smuggling_guild.people.(Category).(index).name = GetMySimpleName(chr);
       pchar.quest.smuggling_guild.people.(Category).(index).location = GetPrettyLocationName(chr.location);
       pchar.quest.smuggling_guild.people.(Category).(index).island = GetIslandNameByLocationID(chr.location);
       chr.Dialog.Filename.SmugglingGuild = Dialog;
       chosen += 1;
     }
   }
}

The same thing happens... if the AbordageMode bit checks for true with this code, then it hangs.

 

[Pieter Boelen]

I really like the way you rewrote that!
Looks so much cleaner than those nested if statements.

I assume that there are zero characters in the game that have Abordage Mode set at all.
Not even sure why that check is in there.
Can only be set through dialog for player officers.

 

[Mere_Mortal]

I assume that there are zero characters in the game that have Abordage Mode set at all.

You might be in for a bit of a surprise.

int q = 0
for (int i=0; i<2000; i++) {
  if (GetAttribute(GetCharacter(i),"AbordageMode") == true) q++ ;
}
Trace(q)

Run that at the console then try explaining why the loop acts like it does, ‘cos I ain’t got the foggiest.

 

[Pieter Boelen]

You might have to stick a 'sti' in there because for some reason all character attributes are saved as strings.

Also, I think you're right that the GetAttribute line should be followed by != instead of == .

 

[Mere_Mortal]

Oh, I did wonder what the difference was with that sti, I just thought it was something I might catch in Tortuga.

Either way, I’m not sure if it matters if the value is 0 or 1, even if it’s a string it should come back as bool if tested that way.

 

[Pieter Boelen]

GetAttribute returns the string "-1" of the attribute is not there.
Otherwise it returns the value of the attribute.
So probably a != "1" is what it needs.

 

[Mere_Mortal]

I think I tried it with GetAttribute and CheckAttribute, with 0, 1, true, false, quoted and unquoted, and with a straight-up chr.abordagemode as well. The results were the same whichever way, but I’ll check it again later to make sure I did try each combination. Surely I got something wrong, just like how it was wrong in the first place, because it simply shouldn’t be acting like that otherwise.

 

[Pieter Boelen]

I'm looking into this one now as well....

 

[Pieter Boelen]

This is quite a tough nut to crack properly, because there are SO many different characters and different types of characters in the game.
The original function allowed all sorts of unintentional characters to "slip through", so I'm trying to tighten the checks now to avoid that.

 

[Mere_Mortal]

Because over 99% of characters have the attribute then it cannot find a match, and so the function will loop forever.

The big question is... who are those characters with no attribute at all? Could they actually be legitimate “clients”?

 

[Pieter Boelen]

I'm definitely coming closer to making this functionality, far, far safer.

There were all sorts of errors that could occur. Fort Commanders and Player Crewmembers as Opium Buyers, for example.
Plus you could get Opium Buyers on locations you couldn't access, such as Isla Mona.

Not to mention that it checked the "officertype" attribute, which is now set for ALL characters as soon as you enter a location.
This basically meant that you could never get an Opium Buyer in a location you had already visited in the game.
The person who made the "Opium" mod should be aware of that though, since HE is the one who added that attribute to all characters in the first place.

This has also led me to find all sorts of mistakes in the character and location init files, as shown here:
Fix in Progress - "Falaise_de_fleur_shore_01" does not exist | PiratesAhoy!
Unconfirmed Bug - Oranjestad No Shipyard | PiratesAhoy!

Because over 99% of characters have the attribute then it cannot find a match, and so the function will loop forever.

The big question is... who are those characters with no attribute at all? Could they actually be legitimate “clients”?

I think this check works:

     if (GetAttribute(chr,"abordagemode") != "1")               continue;
     if(DEBUG_SMUGGLING>2) trace("SMUGGLING GUILD MEMBERS pass boardingmode check");

Apparently ALL characters get that attribute set to "1" when they're initialized.
I'm not sure if there is any point in checking that at all. The only characters that could have it at "0" are your own officers.
And they aren't legitimate "clients" in the first place.

 

[Mere_Mortal]

You don’t know my officers then.

Right, so the ones with no attribute set... I’m guessing they’re either the player’s officers or companions, else they are coastal traffic.

I just scanned their cargo holds and while some of them did have contraband, I saw no opiates.