• New Horizons on Maelstrom
    Maelstrom New Horizons


    Visit our website www.piratehorizons.com to quickly find download links for the newest versions of our New Horizons mods Beyond New Horizons and Maelstrom New Horizons!

Fix in Progress Companion can't find his island

Grey Roger

Grey Roger

Sea Dog
Staff member
Administrator
Storm Modder
An odd bug popped up while I was playing "Tales of a Sea Hawk". I had just escaped from Speightstown with Rabel Yverneau's frigate and was making my way to Port Royale when I engaged a group of three pirate ships. Two were jackass barques which I couldn't catch, partly because my sails were still damaged from the escape and partly because frigates don't do as well as jackass barques when the wind is blowing from the side. But the third ship was a basic barque which eventually surrendered. The captain really did surrender so I made him an officer and put him back in command of his ship. After arriving at Port Royale and going through the shenanigans with Silehard, cursed pirates, being thrown into jail, and more Silehard, I went to the shipyard and sold the barque.

Which should not have been possible. The ship was carrying some coffee, which is contraband in Port Royale. "error.log" showed problems with "islands.c", where 'GetIslandByIndex' had been told to find island with index -1; and with "shipyard.c", where 'CheckForContraband' had failed to determine someone's island. Adding some "traceandlog" commands to various places nailed down the problem to 'GetCharacterCurrentIsland' in "CharacterUtilite.c":
Code: 
int GetCharacterCurrentIsland(ref _refCharacter)
{
// KK -->
   /*int curLocNum = FindLocation(Characters[GetMainCharacterIndex()].location);
   if(curLocNum<0) return -1;
   return GetIslandIdxByLocationIdx(curLocNum);*/
   // PB: Make this work for companion ships while ashore too -->
   string curLoc = "";
   if (CheckAttribute(_refCharacter, "location.from_sea"))               curLoc = _refCharacter.location.from_sea;
   if (curLoc == "" && CheckAttribute(_refCharacter, "location"))       curLoc = _refCharacter.location;
   // PB: Make this work for companion ships while ashore too <--
   int locidx = FindLocation(curLoc);
   if (locidx < 0) return -1;
   return GetIslandIdxByLocationIdx(locidx);
// <-- KK
}
A 'dumpattribute(_refCharacter)' showed that the ex-pirate companion captain was at location "None"! That has index 0, so 'locidx' was set to 0 and 'GetIslandIdxByLocationIdx(locidx)' returned -1.

I added a check that if it can't find an island for 'locidx' and '_refCharacter' is a companion, find the player's island instead - companions tend to be at the same place as you, otherwise they're not companions! With that check in place, 'CheckForContraband' worked correctly and the ship could not be sold with coffee aboard.

The fixed version of "CharacterUtilite.c" will be in the next update.
 
HA! Good catch! :cheers

Grey Roger said:
A 'dumpattribute(_refCharacter)' showed that the ex-pirate companion captain was at location "None"!
Click to expand...
I think "none" is the location ID when you reload from the worldmap to a scene that doesn't have an island in it.
But I assume the last known location of that captain was 3D sailing mode around Jamaica?

Could be that the .location attribute isn't updated for AI captains?
 
The captain's location would have been "None" when I captured him as that was indeed out in deep sea, the result of a worldmap encounter out of range of any island. It's possible that this particular captain's "location" attribute was not updated when I arrived at Jamaica because I didn't dock in the usual manner. And if a companion's "location.from_sea" attribute would normally be set when you dock, it certainly won't have been set on this occasion. When you enter Jamaica waters with Rabel Yverneau and Tobias, you teleport straight to the residence for a chat with Silehard.

So it may be that the bug only bites if you've just completed this part of "Tales of a Sea Hawk", or any other quest or storyline which teleports you straight to land. Even then, you wouldn't notice it unless the companion ship is carrying contraband, you know this, you get to sell it anyway, and you don't have the "Trustworthy" ability - exactly the set of conditions which happened here.
 
Confirmed. Using console, I first ran 'dumpattributes' on my own character to find the captain's index number, then ran 'dumpattributes' on him. "location" was set to "None",
"location.from_sea" was not set at all.

Then I loaded up a later savegame from the same campaign - having left Port Royale, I'd made my way to San Juan to stock up on armour, capturing a pinnace stuffed with silk on the way. Silk being contraband on Puerto Rico, it should have been impossible to sell that ship there, and it was. A 'dumpattributes' on that ship's captain showed that again, "location" was set to "None", but "location.from_sea" was set to "Muelle_port". Attribute "location" shows the character's location on land so is probably set to "None" for companions as they aren't on land (except, presumably, for some quest companions who can simultaneously be a companion and a party officer).

Attribute "location.from_sea" is where the ship is moored; the storyline code sets it for you soon after you're sent to the residence, but doesn't check for companions. The same is probably true for other quests which teleport you to land - the attribute must be set for you so that you can return to your ship, but the game doesn't fail catastrophically if it's not set for companions.
 
Yes, on consideration, that probably has to be true so that quest conditions such as...
Code: 
Pchar.quest.Story_BlazeEscapedFromOxbay.win_condition.l1.location = "Redmond";'
... can work.

And I can see another potential bug, though it probably isn't an issue with any current quests. In "quests_check.c":
Code: 
case "location":
   if(refCharacter.location==condition.location) return !CharacterIsDead(refCharacter);
   return false;
break;
That's fine if it's checking your location. It won't work if you try to set a condition that someone else has to be at a given island. This would be the sea-going equivalent to a check on another character being at a certain land location, e.g. for Artois Voysey to wander off at La Grenade:
Code: 
Pchar.quest.Artois_Voysey.win_condition.l1 = "location";
Pchar.quest.Artois_Voysey.win_condition.l1.character = "Artois Voysey";
Pchar.quest.Artois_Voysey.win_condition.l1.location = "Conceicao_port";
PChar.quest.Artois_Voysey.win_condition = "Artois_missed";
This will happen if you're in La Grenade port and Artois is in your party - he follows you around so his "location" becomes "Conceicao_port". But...
Code: 
PChar.quest.Companion_to_Jamaica.win_condition.l1 = "location";
PChar.quest.Companion_to_Jamaica.win_condition.l1.character = "Insert_Name";
PChar.quest.Companion_to_Jamaica.win_condition.l1.location = "Redmond";
PChar.quest.Companion_to_Jamaica.win_condition = "Companion_to_Jamaica";
... will not work if "Insert_Name" does not have his "location" attribute updated at sea like yours. (So it's just as well that the code to make Nigel Blythe disappear with the ship you gave him triggers when you reach Puerto Rico.)
 
Very true.

Maybe the code where that attribute is set for the player should be found; and updated to set it for NPC ships too.
 
Grey Roger said:
Where is that code to be found?
Click to expand...
I think it is this:
Code: 
void SeaLogin(ref Login)
{
   Trace("SEA: SeaLogin begin");
   int i, j, iShipType, iNumQCGroups, iNShips, iCharacterIndex;
   float x, y, z, ay;
   ref    rCharacter, rGroup, rEncounter;
   aref rRawGroup, arQCGroups, arEncounters;
   string sGName, l;
   bool bDirectSail = CheckAttribute(Login, "DirectSail"); // KK
   bool bLoadSavedGame = CheckAttribute(Login, "LoadAtSea") && !bDirectSail; // KK

   int iRDTSC = RDTSC_B();

   // clear load groups now object
   DeleteAttribute(&LoginGroupsNow, "");

   iStormLockSeconds = 0;
   iNumFantoms = 0;
   bSkipSeaLogin = false;
   bSeaReloadStarted = false;
   bSeaQuestGroupHere = false;
   bIslandLoaded = false;

   fSeaExp = 0.0;
   fSeaExpTimer = 0.0;
   // NK 04-09-21 timeupdate
   if(TIMEUPDATE_BLOCK_LAND)
   {
       AddTimeToCurrent(0, (locTmpTime * TIMESCALAR_LAND)/60);
   }
   locTmpTime = 0.0;
   // NK <--

   Sea_FreeTaskList();
   Encounter_DeleteDeadQuestMapEncounters();

// KK -->
   if (!bDirectSail) {
       // weather parameters
       if (!bLoadSavedGame) {
           WeatherParams.Tornado = false;
           WeatherParams.Storm = false;
       }
       if (CheckAttribute(Login,"Storm")) WeatherParams.Storm = Login.Storm;
       if (CheckAttribute(Login,"Tornado")) WeatherParams.Tornado = Login.Tornado;
       bStorm = sti(WeatherParams.Storm);
       bTornado = sti(WeatherParams.Tornado);
       if (bStorm) iStormLockSeconds = 60;
   }

   // Island
   int iIslandIndex = FindIsland(Login.Island);
   //Trace("Island id = " + Login.Island + ", Island index = " + iIslandIndex);
   string sIslandID = "";
   if (iIslandIndex != -1) sIslandID = Islands[iIslandIndex].id;

   // main character
   ref rPlayer = GetMainCharacter();
   rPlayer.Ship.Stopped = false;
   //DeleteAttribute(rPlayer, "LastSailState"); // NK 05-04-20 so it doesn't say it during loading
   rPlayer.Ship.POS.Mode = SHIP_SAIL;
   rPlayer.location = Login.Island;
Indeed from PROGRAM\SEA_AI\sea.c .

I think it would need something like this:
Code: 
    for (i = 0; i < COMPANION_MAX; i++) {
       iCharacterIndex = GetCompanionIndex(rPlayer, i);
       if (iCharacterIndex < 0) continue;
       rCharacter = GetCharacter(iCharacterIndex);
       rCharacter.location = Login.Island;
   }

There are a bunch of other loops it could be worked into as well, such as:
Code: 
    for (c = 0; c <= GetCompanionQuantity(rPlayer); c++) {
       rCharacter = GetCharacter(GetCompanionIndex(rPlayer, sti(c)));
       // PB: To make sure this is gone -->
       DeleteAttribute(rCharacter, "Ship.Sink");
       DeleteAttribute(rCharacter, "Ship.Sails.Delay");
       DeleteAttribute(rCharacter, "Ship.Unstable");
       DeleteAttribute(rCharacter, "Ship.Capsize");
       DeleteAttribute(rCharacter, "Ship.Tack");
       // PB: To make sure this is gone <--
   }

Or, going even further as an attempt to add it to ALL ships at sea:
Code: 
    iNShips = sti(Login.Ship.Quantity);
   for (i = 0; i < iNShips; i++)
   {
       l = "l" + i;
       if(!CheckAttribute(&Login,"Ship."+(l))) continue; // Screwface
       iCharacterIndex = sti(Login.Ship.(l).idx);
       if (iCharacterIndex < 0) continue;
       rCharacter = GetCharacter(iCharacterIndex);
       rCharacter.location = Login.Island;
   }
Note that this would have to be a NEW loop, rather than the very similar existing one because that only gets triggered 'if (bLoadSavedGame)'.
 
Pieter Boelen said:
I think it would need something like this:
Code: 
    for (i = 0; i < COMPANION_MAX; i++) {
       iCharacterIndex = GetCompanionIndex(rPlayer, i);
       if (iCharacterIndex < 0) continue;
       rCharacter = GetCharacter(iCharacterIndex);
       rCharacter.location = Login.Island;
   }
Click to expand...
Thanks! I'll see if that does anything useful - it certainly doesn't look as though it could do any harm.

There are a bunch of other loops it could be worked into as well, such as:
Code: 
    for (c = 0; c <= GetCompanionQuantity(rPlayer); c++) {
       rCharacter = GetCharacter(GetCompanionIndex(rPlayer, sti(c)));
       // PB: To make sure this is gone -->
       DeleteAttribute(rCharacter, "Ship.Sink");
       DeleteAttribute(rCharacter, "Ship.Sails.Delay");
       DeleteAttribute(rCharacter, "Ship.Unstable");
       DeleteAttribute(rCharacter, "Ship.Capsize");
       DeleteAttribute(rCharacter, "Ship.Tack");
       // PB: To make sure this is gone <--
   }
Click to expand...
I'm not sure what good it would do here - the player's location isn't being set, so there's no reason to set the companions' locations either. Anyway, it's all part of function "SeaLogin", so won't companion locations already be set by the first additional loop?

Or, going even further as an attempt to add it to ALL ships at sea:
Click to expand...
That should not be necessary. The only reason for adding it to companions is in case someone tries to use "location" as a condition for a quest and specifies a companion as the character to be checked. It is unlikely that anyone will want to use a non-companion NPC ship for such a check - those ships won't follow you around, so they'll be wherever the quest put them. (Apart from random NPC's with random ID's which, being random, can't be checked for quest purposes anyway.)
 
Grey Roger said:
I'm not sure what good it would do here - the player's location isn't being set, so there's no reason to set the companions' locations either. Anyway, it's all part of function "SeaLogin", so won't companion locations already be set by the first additional loop?
Click to expand...
It's either in the one spot; or in the other. No point doing it twice.
When I went through the code, I though perhaps that "To make sure this is gone" might be a cleaner place for it; because it's effectively the same loop, except it's already there.

Grey Roger said:
That should not be necessary. The only reason for adding it to companions is in case someone tries to use "location" as a condition for a quest and specifies a companion as the character to be checked. It is unlikely that anyone will want to use a non-companion NPC ship for such a check - those ships won't follow you around, so they'll be wherever the quest put them. (Apart from random NPC's with random ID's which, being random, can't be checked for quest purposes anyway.)
Click to expand...
Very true.
I was mainly wondering about a ship captured around an island and taken for yourself and then missing the attribute.
But I'm not sure if that would even go wrong in the same way. And even if it does, it'd probably be VERY unlikely.
 
Pieter Boelen said:
It's either in the one spot; or in the other. No point doing it twice.
When I went through the code, I though perhaps that "To make sure this is gone" might be a cleaner place for it; because it's effectively the same loop, except it's already there.
Click to expand...
Perhaps, but just to be safe, make sure that companions have their "location" set at the same place as the player. That way, if it needs to change for the player, whoever is doing the changing can see that it needs to change for the companions as well.

However, I do have to wonder about something in that second loop:
Code: 
for (c = 0; c <= GetCompanionQuantity(rPlayer); c++)
Should that not be:
Code: 
for (c = 0; c <= COMPANION_MAX; c++)
Otherwise, if you have companions in slots 1 and 3 but not in 2, companion 2 might be checked unnecessarily and companion 3 not checked at all.

I was mainly wondering about a ship captured around an island and taken for yourself and then missing the attribute.
But I'm not sure if that would even go wrong in the same way. And even if it does, it'd probably be VERY unlikely.
Click to expand...
That should not be an issue. This is for the benefit of a companion who is already with you and you're supposed to bring to a given island, e.g.
Code: 
PChar.quest.Companion_to_Jamaica.win_condition.l1 = "location";
PChar.quest.Companion_to_Jamaica.win_condition.l1.character = "Insert_Name";
PChar.quest.Companion_to_Jamaica.win_condition.l1.location = "Redmond";
PChar.quest.Companion_to_Jamaica.win_condition = "Companion_to_Jamaica";
If you're checking for capturing a ship then it would be done by checking the death of the captain. If, for any reason, you need to check that you're capturing the ship at the correct island and the target ship can somehow move around, you'd check it with a "location" check on the player.
 
Grey Roger said:
Perhaps, but just to be safe, make sure that companions have their "location" set at the same place as the player. That way, if it needs to change for the player, whoever is doing the changing can see that it needs to change for the companions as well.
Click to expand...
Since companion #0 IS the player, you might even be able to remove 'rPlayer.location = Login.Island;' altogether as that would be covered again by the loop right after.

Grey Roger said:
However, I do have to wonder about something in that second loop:
Code: 
for (c = 0; c <= GetCompanionQuantity(rPlayer); c++)
Should that not be:
Code: 
for (c = 0; c <= COMPANION_MAX; c++)
Otherwise, if you have companions in slots 1 and 3 but not in 2, companion 2 might be checked unnecessarily and companion 3 not checked at all.
Click to expand...
In that case:
Code: 
   for (i = 0; i < COMPANION_MAX; i++) {
      iCharacterIndex = GetCompanionIndex(rPlayer, i);
      if (iCharacterIndex < 0) continue;
      rCharacter = GetCharacter(iCharacterIndex);
      // PB: To make sure this is gone -->
      DeleteAttribute(rCharacter, "Ship.Sink");
      DeleteAttribute(rCharacter, "Ship.Sails.Delay");
      DeleteAttribute(rCharacter, "Ship.Unstable");
      DeleteAttribute(rCharacter, "Ship.Capsize");
      DeleteAttribute(rCharacter, "Ship.Tack");
      // PB: To make sure this is gone <--
   }
I always had the impression both work equally; but indeed for shore officers the 'slot' number can be quite relevant, so who knows... might apply to companions too.
 
Confirmed, the original version does not work properly. I added a couple of trace statements: if 'GetCompanionIndex(rPlayer, c)' was positive then show the character's name, otherwise show the slot number of the faulty captain. As I have a savegame in a port where a couple of ships are berthed, I relaunched one of them into slot 3 (the relaunch interface calls it slot 4 because it counts from 1). Then I put to sea, quit, and checked "compile.log". It did exactly what I expected. 'GetCompanionQuantity(rPlayer)' is 2 (me plus one companion), so it cycled from 0 to 2, reported my name, reported slots 1 and 2 as faulty, and didn't report slot 3 at all.

The version with your fix, going up to COMPANION_MAX and skipping any slot with a negative index, works. Similar trace statements showed checks on me and the companion, with no reports on slots 1 and 2 - I'd put the traces just before '// PB: To make sure this is gone -->'.

The fixed version is going in the update.
 
You must log in or register to reply here.
Back
Top