Fixed Strange Island Code

[Pieter Boelen]

I found some weird code in islands.c and rewrote it accordingly:

string FindIslandByLocation(string locID)//MAXIMUS
{
   // PB: Simplified -->
   if(locID != "")
   {
     int ind = FindIsland(locID);
     if (ind >= 0) // If location is an existing island
     {
       return locID; // Return that
     }
     else
     {
       ind = FindLocation(locID);
       if (ind >= 0) // You are in a valid location ashore
       {
         ref lcn = &locations[ind];
         if(CheckAttribute(lcn,"island"))   return lcn.island; // Return location island
         if(CheckAttribute(lcn,"townsack"))   return GetIslandIDFromTown(lcn.townsack); // If missing, return town's island
       }
     }
   }
   return "";
   // PB: Simplified <--
/*   if(locID=="" || FindLocation(locID)==-1)
   {
     if(CheckAttribute(&characters[GetMainCharacterIndex()],"location.from_sea")) return Islands[FindIslandBySeaLocation(characters[GetMainCharacterIndex()].location.from_sea)].id;
     return "";
   }
   if(CheckAttribute(&locations[FindLocation(locID)],"townsack")) return GetIslandIDFromTown(GetCurrentTownID()); // PB: This is DEFINITELY wrong!
   if(CheckAttribute(&locations[FindLocation(locID)],"island")) return locations[FindLocation(locID)].island;
   if(FindIslandBySeaLocation(FindSeaShoreForLocation(locID))!=-1) return Islands[FindIslandBySeaLocation(FindSeaShoreForLocation(locID))].id;
   return "";*/
}

@Levis, can you think of ANY reason why that old code was doing all those things?
It seems quite complex, yet doesn't make much sense to me.
And there is one line that was DEFINITELY wrong because it ended up looking at a different island than the one for the location you're asking about.

EDIT: Probably better to reverse the logic on this one and check first if you are at sea.

 

[Pieter Boelen]

Also this from kam_shipberthing_ship.c:

{
   // PB: Simplified -->
   ref PChar = GetMainCharacter();
   if (CheckAttribute(PChar, "location"))
   {
     return FindIsland(FindIslandByLocation(PChar.location)); // PB: This uses strings as input and output, but needs to return the island index here
   }
   else
   {
     return 0;
   }
   // PB: Simplified <--
   
/*   ref lcn = &locations[FindLocation(FindLoadedLocation())];
   //if (FindIslandByLocation(lcn.id) == "KhaelRoa") return FindIsland("KhaelRoa"); // PB: Fix this properly by adding island attributes to all Khael Roa locations
   // Screwface
   if(Checkattribute(lcn,"island")) return FindIsland(lcn.island);
   // This way causes errors on seashores
   int townidx = FindCurrentPort();
   return FindIsland(Towns[townidx].island);*/
}

 

[Levis]

sounds like good fixes.
got no idea what they where thinking when programming it like that.

About the first fix. Would the findisland be neccesary
Don't all locations have an island or something like that assigned to it?

 

[Pieter Boelen]

All shore locations should have the .island attribute, yes. That is what the function now uses.
It should never get as far as the .townsack check. But then.... some locations aren't set up right.
Such as a whole bunch of Khael Roa ones where I added their island attributes just now because they weren't already there.

Still... I'd rather get an error message on a missing attribute so that we can add it,
rather than the code being modified to perform all sorts of fantastic work-arounds.
If a bug is to be fixed, the best way to do that is to do it properly.

It does have a check for island first, because pchar.location can be an island ID as well.

 

[Levis]

ok, maybe add some debug traces (for now) to warn us when it actually goes to the townsack one etc?

Shouldnt you use the find island by ID btw?
I believe there is such a function also.

 

[Pieter Boelen]

ok, maybe add some debug traces (for now) to warn us when it actually goes to the townsack one etc?

How's this?

  if(locID != "")
   {
     int ind = FindIsland(locID);
     if (ind >= 0) // If location is an existing island
     {
       return locID; // Return that
     }
     else
     {
       ind = FindLocation(locID);
       if (ind >= 0) // You are in a valid location ashore
       {
         ref lcn = &locations[ind];
         if(CheckAttribute(lcn,"island"))   return lcn.island; // Return location island
         TraceAndLog("FindIslandByLocation ERROR: Please post your compile.log file at piratesahoy.net!");
         trace("Location " + lcn.id + " does not have .island attribute!");
         if(CheckAttribute(lcn,"townsack"))   return GetIslandIDFromTown(lcn.townsack); // If missing, return town's island
       }
     }
     trace("BAD ERROR: No island found");
   }
   return "";

Shouldnt you use the find island by ID btw?
I believe there is such a function also.

This one, you mean?

ref GetIslandByID(string sIslandID)

That one returns a reference; I want the island ID itself.

 

[Pieter Boelen]

Looks like at least one of those checks WAS needed for your ship deck after reloading from sea:

  // PB: Simplified -->
   ref PChar = GetMainCharacter();
   if(locID != "")
   {
     int ind = FindIsland(locID);
     if (ind >= 0) // If location is an existing island
     {
       return locID; // Return that
     }
     else
     {
       ind = FindLocation(locID);
       if (ind >= 0) // You are in a valid location ashore
       {
         ref lcn = &locations[ind];
         if(CheckAttribute(lcn,"island"))         return lcn.island;               // Return location island
         if(CheckAttribute(lcn,"townsack"))         return GetIslandIDFromTown(lcn.townsack);   // If missing, return town's island
         if(CheckAttribute(PChar,"location.from_sea"))   return PChar.location.from_sea;         // For ship deck
       }
     }
     TraceAndLog("FindIslandByLocation ERROR: Please post your compile.log file at piratesahoy.net!");
     trace("No island found for " + lcn.id);
   }
   return "";
   // PB: Simplified <--

 

[Pieter Boelen]

This is now included in the ZIP attached here: http://www.piratesahoy.net/threads/build-14-beta-3-5-internal-wip-for-testing.24817
If there is an on-screen log message about an island-related error, please let us know.

 

[Levis]

Nobody got any errors yet?

 

[Levis]

Moved, when people do notice something we can put it back again.