Fixed Crash from Cargo Quest: Martinho Guterres in Sao Jorge

Levis · Sep 14, 2016

@Pieter Boelen you can probably remove this check from the while loop then

.

Pieter Boelen · Sep 14, 2016

Turns out the very same error is also in the 'GenerateGoodForTrade' function in quests_common.c .
That suggests the whole generation system there makes NO SENSE!
It basically looks for a nation first, then a commodity and then a town.
And it does this completely independently from each other, so that there does not necessarily need to be any actual logical link between these variables.

Of course the CORRECT way would be to:
1. Check all available towns
2. Choose one at random
3. Return the nation from that
4. Find an import good for that town

I also saw some code from 2005 that basically suggested this stuff was being modded, with a hack in place, because someone else was going to update it.
Clearly that never happened, since it is still in there.

Anyway, the main problem is the actual crash. And that can be fixed by making the following changes....

In PROGRAM\QUESTS\quests_common.c find:

Code:

int GenerateGoodForTrade(int FromNat, int ToNat, ref fromprice, ref toprice) // NK add prices 05-05-12
{
   //int fstoreidx = GetTownStoreIndex(GetTownIDFromGroup(Stores[GetCharacterCurrentStore(GetMainCharacter())].group));
   int fstoreidx = GetTownStoreIndex(GetCurrentTownID());
   //trace("from " + fstoreidx + " of " + GetCurrentTownID());
   string destCol;
   string friendlyTowns = "";
   for(int j=0; j<TOWNS_QUANTITY; j++)
   {
     if(!CheckAttribute(&Towns[j],"id")) continue;
     if(!CheckAttribute(&Towns[j],"nation")) continue;
     if(CheckAttribute(&Towns[j],"skiptrade") && Towns[j].skiptrade==true) continue;//MAXIMUS: added for some towns (such as St. John's on Antigua)
     if(Towns[j].nation!=ToNat) continue;
     friendlyTowns = StoreString(friendlyTowns,Towns[j].id);
   }
   if(friendlyTowns!="")
   {
     destCol = GetRandSubString(friendlyTowns);
     while(destCol=="" || HasSubStr(destCol,",") || GetIslandIDFromTown(destCol)==GetIslandIDFromTown(GetCurrentTownId())) { destCol = GetRandSubString(friendlyTowns); }
   }
   else
   {
     switch (ToNat)
     {
       case FRANCE: destCol = FRA_COLONY; break;
       case SPAIN: destCol = SPA_COLONY; break;
       case HOLLAND: destCol = HOL_COLONY; break;
       case PORTUGAL: destCol = POR_COLONY; break;
       case ENGLAND:  destCol = RED_COLONY; break;
     }
   }

Replace with:

Code:

int GenerateGoodForTrade(int FromNat, int ToNat, ref fromprice, ref toprice) // NK add prices 05-05-12
{
   //int fstoreidx = GetTownStoreIndex(GetTownIDFromGroup(Stores[GetCharacterCurrentStore(GetMainCharacter())].group));
   int fstoreidx = GetTownStoreIndex(GetCurrentTownID());
   //trace("from " + fstoreidx + " of " + GetCurrentTownID());
   string destCol;
   string friendlyTowns = "";
   for(int j=0; j<TOWNS_QUANTITY; j++)
   {
     if(!CheckAttribute(&Towns[j],"id")) continue;
     if(!CheckAttribute(&Towns[j],"nation")) continue;
     if(CheckAttribute(&Towns[j],"skiptrade") && Towns[j].skiptrade==true) continue;//MAXIMUS: added for some towns (such as St. John's on Antigua)
     if(Towns[j].nation!=ToNat) continue;
     if(GetIslandIDFromTown(Towns[j].id) == GetIslandIDFromTown(GetCurrentTownID())) continue;
     friendlyTowns = StoreString(friendlyTowns,Towns[j].id);
   }
   if(friendlyTowns!="")
   {
     destCol = GetRandSubString(friendlyTowns);
     while(destCol=="" || HasSubStr(destCol,",")) { destCol = GetRandSubString(friendlyTowns); }
   }
   else
   {
     switch (ToNat)
     {
       case FRANCE: destCol = FRA_COLONY; break;
       case SPAIN: destCol = SPA_COLONY; break;
       case HOLLAND: destCol = HOL_COLONY; break;
       case PORTUGAL: destCol = POR_COLONY; break;
       case ENGLAND:  destCol = RED_COLONY; break;
     }
   }

Then in PROGRAM\MAXIMUS_functions.c find:

Code:

string GenerateTradeQuest(ref pchar, int iTradeNation, int iTradeGoods, float fprice, float tprice, bool bMain)// moved here from traders dialogs
{
   int i, iQuantityGoods, iMoney, iTradeExp, iDaysExpired, chridx;
   float fDistance, fWeight, pricediff;
   string sTown = "";
//MAXIMUS -[such functions must be universal (IMHO), if we are planning to make a real Caribbean in the future]->
   string friendlyTowns = "";
   string pirateTowns = "";
   ref chr;
   for(i = 0; i < TOWNS_QUANTITY; i++)
   {
     if(!CheckAttribute(&Towns[i],"id")) continue;
     if(!CheckAttribute(&Towns[i],"nation")) continue;
     if(GetAttribute(&Towns[i],"skiptrade") == true) continue;//MAXIMUS: added for some towns (such as St. John's on Antigua)
     if(sti(Towns[i].nation) != iTradeNation) continue;
     friendlyTowns = StoreString(friendlyTowns,Towns[i].id);
   }
   if (friendlyTowns != "") {
     sTown = GetRandSubString(friendlyTowns);
     while (sTown == "" || HasSubStr(sTown, ",") || GetIslandIDFromTown(sTown) == GetIslandIDFromTown(GetCurrentTownId()))
     {
       sTown = GetRandSubString(friendlyTowns);
     }
   } else {
     sTown = "Quebradas Costillas"; //MAXIMUS: Quebradas Costillas - why some trader can't trade with pirates?
   }

Replace with:

Code:

string GenerateTradeQuest(ref pchar, int iTradeNation, int iTradeGoods, float fprice, float tprice, bool bMain)// moved here from traders dialogs
{
   int i, iQuantityGoods, iMoney, iTradeExp, iDaysExpired, chridx;
   float fDistance, fWeight, pricediff;
   string sTown = "";
//MAXIMUS -[such functions must be universal (IMHO), if we are planning to make a real Caribbean in the future]->
   string friendlyTowns = "";
   string pirateTowns = "";
   ref chr;
   for(i = 0; i < TOWNS_QUANTITY; i++)
   {
     if(!CheckAttribute(&Towns[i],"id")) continue;
     if(!CheckAttribute(&Towns[i],"nation")) continue;
     if(GetAttribute(&Towns[i],"skiptrade") == true) continue;//MAXIMUS: added for some towns (such as St. John's on Antigua)
     if(sti(Towns[i].nation) != iTradeNation) continue;
     if(GetIslandIDFromTown(Towns[i].id) == GetIslandIDFromTown(GetCurrentTownID())) continue;
     friendlyTowns = StoreString(friendlyTowns,Towns[i].id);
   }
   if (friendlyTowns != "") {
     sTown = GetRandSubString(friendlyTowns);
     while (sTown == "" || HasSubStr(sTown, ","))
     {
       sTown = GetRandSubString(friendlyTowns);
     }
   } else {
     sTown = "Quebradas Costillas"; //MAXIMUS: Quebradas Costillas - why some trader can't trade with pirates?
   }

That is confirmed to solve the crash.

The fact that this whole process makes no sense remains though.
But I suppose that is not necessarily a big issue; it just means the assignments may not make much logical sense.

Levis said:
@Pieter Boelen you can probably remove this check from the while loop then .

Already done.

Redbeard · Sep 14, 2016

I should have clarified that it only happens around 50% of the time, luckily it seems you smart fellows have figured everything out!

Thanks for the help!

Pieter Boelen · Sep 14, 2016

Redbeard said:
I should have clarified that it only happens around 50% of the time, luckily it seems you smart fellows have figured everything out!

You DID actually say that. I just didn't notice until I tested the problem, couldn't replicate it, and then re-read your initial report.
That is when I found out you already mentioned it was random, which got me to test further and find the actual culprit.

So don't nay-say your own reporting skills!

Levis · Sep 15, 2016

@Pieter Boelen could you make a build 15 topic about this so we can then take a good look at it?
If I'm going to revamp the trading system I want to take this into account too.

Pieter Boelen · Sep 15, 2016

Levis said:
@Pieter Boelen could you make a build 15 topic about this so we can then take a good look at it?
If I'm going to revamp the trading system I want to take this into account too.

Done: Planned Feature - Rework Cargo Quest Logic | PiratesAhoy!

Tingyun · Sep 15, 2016

Should we go ahead and use Pieter's quickfix for the crash in the meantime? I've made the edits Pieter listed to my own files, and uploaded here. Should we post to new content?

EDIT Maximus is in general PROGRAM folder, the quests_common is in PROGRAM/QUESTS

Pieter Boelen · Sep 15, 2016

Tingyun said:
Should we go ahead and use Pieter's quickfix for the crash in the meantime?

Yes, you should! :cheeky

Redbeard · Sep 15, 2016

Haha WOW! I figured I could just download Tingyun's files and replace my own with them. HUUUUUGE mistake lol. Gives me a runtime error engine.exe abnormal program termination. Completely ruined my install of New Horizons. Now I need to replace those with the stock ones from the mod install.

A warning to anyone who might think of doing the same.

Tingyun · Sep 15, 2016

@Redbeard Yes, the files are for the latest Beta 4.1 September patch, recently released. Generally, many of the fixes released in the next few days will very possibly be incompatible with the July 28th version, since we are often working to fix things that have changed since the latest patch, and many of the files have changed since then anyway.

You can use winmerge to fix your old July 28th files by only merging the change Pieter mentions if you like, or manually search for and replace the parts Pieter mentioned. Otherwise, it will have to wait until you update to the latest Beta patch.

Pieter Boelen · Sep 15, 2016

That's why I just posted the changes to make manually.
Bit more work and a bit more tricky, but bypasses any incompatibility issues.

Redbeard · Sep 15, 2016

Oh ok. Would these changes be new game enforced like said september patch?

Pieter Boelen · Sep 15, 2016

No, the code changes I mentioned here work immediately on your savegame.
That's how I confirmed it works.

Tingyun · Sep 15, 2016

@Levis Maximus is in general PROGRAM folder, the quests_common is in PROGRAM/QUESTS

Levis · Sep 16, 2016

Can someone confirm the bug is solved?

Pieter Boelen · Sep 16, 2016

Levis said:
Can someone confirm the bug is solved?

I can confirm it because I tested it before posting. :cheeky

Fixed Crash from Cargo Quest: Martinho Guterres in Sao Jorge

Levis

Find(Rum) = false;

Pieter Boelen

Navigation Officer

Redbeard

Sailor

Pieter Boelen

Navigation Officer

Levis

Find(Rum) = false;

Pieter Boelen

Navigation Officer

Tingyun

Corsair

Attachments

Pieter Boelen

Navigation Officer

Redbeard

Sailor

Tingyun

Corsair

Pieter Boelen

Navigation Officer

Redbeard

Sailor

Pieter Boelen

Navigation Officer

Tingyun

Corsair

Levis

Find(Rum) = false;

Pieter Boelen

Navigation Officer