Needs Testing [WIP] Generate Quest Ship Logic Error

ANSEL · Sep 23, 2016

After installing @Levis new ZIP, I got this message: ERROR - Force_GetShipType: unable to find ship, returning a default ship instead. I don't remember seeing this before.

Pieter Boelen · Sep 23, 2016

That is a failsafe in the code kicking in and should indeed not happen, or only very, VERY rarely.

Does that happen all the time? Or in specific situations?

Thanks for reporting! :onya

Levis · Sep 23, 2016

I think these ships are used for the virtual smugglers which supply islands with smuggling goods. So I don't think we have to worry to much about it. Probably a specific tier of ship was asked for a specific nation in a specific time which didn't exists.
If the problem appears more often please do tell.

Personally I'm more concerned about the MISSING ITEM ID errors in the compile log and the error log with missing itemID's

Pieter Boelen · Sep 23, 2016

Levis said:
I think these ships are used for the virtual smugglers which supply islands with smuggling goods.

Huh, what? I am not aware of any such ships existing.
The only ships actually supplying smuggling goods to islands are the PLAYER SHIPS.
"Virtual smugglers" don't even exist. o_O

Levis said:
Probably a specific tier of ship was asked for a specific nation in a specific time which didn't exists.

That is, of course, exactly what happens:

Code:

ERROR - Force_GetShipType: unable to find ship, returning a default ship instead
ERROR - Force_GetShipType: Maxclass = 1, Minclass = 3, Per = Colonial Powers, Nat = Pirate, Type = War

With that extra line, the problem becomes quite obvious like this:
The game is trying to find a Pirate ship with at least Tier 3, which obviously doesn't exist as Pirates are limited to Tier 4 and below.

Now the question is: What function can possibly ask for such a ship to be generated?
I'm pretty sure that in the 28 July 2016 version, NO function would ever do that because I put failsafes in place specifically to prevent that.

That means this is most definitely a VERY REAL PROBLEM with (apparently) the 13 September 2016 update.

Levis said:
Personally I'm more concerned about the MISSING ITEM ID errors in the compile log and the error log with missing itemID's

Indeed that looks concerning too. Should probably be a separate Bug Tracker entry.

ANSEL · Sep 23, 2016

@Levis @Pieter Boelen in this case it is a Governors Quest ship , havent done the quest itself yet!

Pieter Boelen · Sep 23, 2016

ANSEL said:
@Levis @Pieter Boelen in this case it is a Governors Quest ship , havent done the quest itself yet!

@Grey Roger: Could you post your quests_common.c file, please? I'm specifically interested in an older version (based on the 28 July 2016 one).

@Levis: You completely rewrote the "Quest Captain" system as well, right?

Levis · Sep 23, 2016

Pieter Boelen said:
@Grey Roger: Could you post your quests_common.c file, please? I'm specifically interested in an older version (based on the 28 July 2016 one).

@Levis: You completely rewrote the "Quest Captain" system as well, right?

I rewrote the captain system but didn't touch the ship.... the ship is an input for my captain generation...

Here is the function as it looks now

Code:

void GenerateQuestShip(string character_id, int iNation) // KK
{
    //Check if the character is an enemy
    bool isEnemy = false;
    if(character_id == "Quest pirate" ) isEnemy = true;
    if(character_id == "Convoy pirate") isEnemy = true;

    //Get the captain which is only half setup
    ref rCaptain = characterFromID(character_id);
   
    //Determine the nation of the ship etc
    if(isEnemy) iNation = LotHostileNation(iNation);
    else        iNation = iNation;
   
    //Extract the nation and fantomtype from it
    string sFantomType = "war";
    if( iNation >= NATIONS_QUANTITY)    iNation = PIRATE;
    if( iNation == PIRATE)                sFantomType = "pirate";

    //Determine the max and minclass of the ship
    int maxclass, minclass, iShipType;
    maxclass = GetShipMinClassForSkills(sti(rCaptain.skill.Leadership), sti(rCaptain.skill.Sailing));    // TODO: Change this so we determine it on something else
    if(sti(rCaptain.nation) == PIRATE             && maxclass < MAXPIRATECLASS)            maxclass = MAXPIRATECLASS;
    if(HasSubStr(character_id,"Coastal_Captain") && maxclass < MAXCOASTGUARDCLASS)        maxclass = MAXCOASTGUARDCLASS;
    if(GetCurrentPeriod() <= PERIOD_EARLY_EXPLORERS || GetCurrentPeriod() >= PERIOD_NAPOLEONIC)
    {
        if(iNation == HOLLAND        && maxclass < 3)                        maxclass = 3;
        if(iNation == PORTUGAL       && maxclass < 3)                        maxclass = 3;
    }
    minclass =            maxclass + 2;
    if(minclass > 8)    minclass = 8;

    //Make the ship
    iShipType = Force_GetShipType(maxclass, minclass, "War", iNation);
   
    //Generate the captain
    if(DEBUG_CAPTAIN_CREATION>1) Trace("CAPTAIN CREATION: In function GenerateQuestShip");
    rCaptain = LAi_Create_Captain(rCaptain, sFantomType, iShipType, iNation); //Levis new function to create a captain

    if(character_id == "Quest pirate")    // PB: For Governor Quest Ships only
    {
        //NK edit trademoney
        ref Shiptype = GetShipByType(iShipType);
        aref arship; makearef(arship, rCaptain.ship);
        ref pchar = GetMainCharacter();
        float shipmult = 5.0 * sqrt(intRet(sti(Shiptype.CannonsQuantity),sti(Shiptype.CannonsQuantity),1) * sti(GetLocalShipAttrib(arship, Shiptype, "HP")) * stf(GetLocalShipAttrib(arship, Shiptype, "SpeedRate"))); // PRS3
        float commult = 1.0 + makeint(CalcCharacterSkill(pchar, SKILL_COMMERCE))/20.0;

        int iTradeMoney = makeint(sqrt(sti(rCaptain.rank)) * shipmult * commult/25)*25;
        if(iTradeMoney < 100) iTradeMoney = 100;

        pchar.quest.generate_kill_quest = "begin";
        pchar.quest.generate_kill_quest.money = iTradeMoney;
        pchar.quest.generate_kill_quest.town = GetCurrentTownID();//MAXIMUS
        pchar.quest.generate_kill_quest.destination = Islands[GetCharacterCurrentIsland(Pchar)].id;
        pchar.quest.generate_kill_quest.nation = iNation; // KK // MAXIMUS
        pchar.quest.generate_kill_quest.shipname = rCaptain.Ship.Name;
    }
}

Pieter Boelen · Sep 23, 2016

In that case, the code to prevent this from happening is still in place:

Code:

if(sti(rCaptain.nation) == PIRATE  && maxclass < MAXPIRATECLASS)  maxclass = MAXPIRATECLASS;

So how can it still happen then? :shock

Also:

Code:

TODO: Change this so we determine it on something else

What is wrong with determining the maximum Tier of ship the captain can command based on his own skills?
Or you propose just using the functions to check skills instead of having it hardcoded like that?
That would indeed be a good idea and should be easy enough.

Plus:

Code:

rCaptain = LAi_Create_Captain(rCaptain, sFantomType, iShipType, iNation); //Levis new function to create a captain

Why do you generate the captain AFTER the captain's skills have been checked to see what ship he can command?
I seem to remember deliberately generating the captain first so that checking his skills afterwards works properly.

In fact, I had to add a hack into your Levelling system to ensure that by the time the captain's skills are checked, he has ALREADY been through the Levelling system.
Otherwise we only got tiny tubs as targets.

Levis · Sep 23, 2016

@Pieter Boelen because in encounters the ship is determined first based on other paramters (the parameters of the encounter) and the captain is based on that. So to have a good captain I take the ship which is generated and base the captain off that.
In this case a character is already provided for this function so I asumed a captain was send to this function already so I could first generate the ship based on the captain send to this function so I know for sure I don't screw up the ships which are generated. After that I generated a good captain with all things set etc.
I see I forgot to add a check here to see if it's a quest character and if so skip the generation.
I have no idea where this function is used and hence i did it in this way to make sure I don't change any of the ship generation itself.

Pieter Boelen · Sep 23, 2016

Levis said:
I see I forgot to add a check here to see if it's a quest character and if so skip the generation.

Of course it is a quest character! It is only ever a quest character in that function. :shock

Levis said:
I have no idea where this function is used and hence i did it in this way to make sure I don't change any of the ship generation itself.

It it used for the following purposes:
- Governor Ship Hunting Quests
- Escort Quest Enemy
- Treasure Quest Enemy
- Smuggling Coast Guard
- Crewmembers on Shore Hostile Captains

Levis said:
because in encounters the ship is determined first based on other paramters (the parameters of the encounter) and the captain is based on that. So to have a good captain I take the ship which is generated and base the captain off that.

I did predict that would be the biggest issue, didn't I?
The 'GenerateQuestShip' function worked correctly, but generated the captain first and then the ship.
All other functions generate the ship first and then the captain.

If you want to update the 'GenerateQuestShip' function to also work that way, then you must think of a completely different way of determining the ship tier to use.
As you have it now, you end up using the previous time this character ID was used to determine the current encounter.
So the previous time, the captain was NOT a Pirate, so this doesn't get triggered:

Code:

if(sti(rCaptain.nation) == PIRATE  && maxclass < MAXPIRATECLASS)  maxclass = MAXPIRATECLASS;

But this time he IS a Pirate, so he can be generated with completely the wrong ship.

Grey Roger · Sep 23, 2016

Pieter Boelen said:
@Grey Roger: Could you post your quests_common.c file, please? I'm specifically interested in an older version (based on the 28 July 2016 one).

It's from the 28th July installer but is dated 16th July.

Pieter Boelen · Sep 23, 2016

Grey Roger said:
It's from the 28th July installer but is dated 16th July.

Thanks very much! That's exactly what I needed. :doff

There you can see the behaviour I described:

Code:

void GenerateQuestShip(string character_id, int iNation) // KK
{
 bool isEnemy = false;
 if(character_id == "Quest pirate" ) isEnemy = true;
 if(character_id == "Convoy pirate") isEnemy = true;
 InitQuestCaptain(character_id, iNation, isEnemy); // LDH 05Sep06 // KK

So the first thing that function does is to generate a new captain character:

Code:

void InitQuestCaptain(string QuestCharacter, int iNation, bool isEnemy)
{
 ref ch = CharacterFromID(QuestCharacter);
 int idx = sti(ch.index);

 int CharacterNation;
 if(isEnemy) CharacterNation = LotHostileNation(iNation);
 else CharacterNation = iNation;

 DeleteAttribute(ch, "");

 ch.id = QuestCharacter;
 ch.index = idx;

This is missing from the current version and therefore it ends up reusing the character from the previous time that function was called.

Probably the ONLY reason why the new version works at all is because these are quest characters who already have skills defined (probably 1 for everything).
But it is definitely no longer working how it is meant to work. :no

Levis · Sep 23, 2016

Guess more work for me to do

.
I will take a look at it today when I'm home and see if I can think up a solution. Till then I'm open for sugestions.

Pieter Boelen · Sep 23, 2016

Levis said:
I will take a look at it today when I'm home and see if I can think up a solution. Till then I'm open for sugestions.

You'd need to determine an appropriate Ship Tier for the encounter, without using its future captain.
So instead of calculating the captain level based on the player level OR player ship tier, you have to determine a formula that returns the encounter ship tier instead.
Then you can generate the ship first and then make a brand new captain for that ship.

Levis · Sep 23, 2016

Pieter Boelen said:
You'd need to determine an appropriate Ship Tier for the encounter, without using its future captain.
So instead of calculating the captain level based on the player level OR player ship tier, you have to determine a formula that returns the encounter ship tier instead.
Then you can generate the ship first and then make a brand new captain for that ship.

The first line is hard the rest isn't a problem...

I'm thinking of changing the argument to be:

GenerateQuestShip(int minlevel, int maxlevel, int iNation)

and then at all places where this function is called it will determine the min and max level.
I think that would make it a easier function to use for other aswell.

Pieter Boelen · Sep 23, 2016

What are 'minlevel' and 'maxlevel' there?

Certainly a more reusable function would be good. But for the current 'GenerateQuestShip' functionality, I can't think why those inputs would be needed.
Maybe make a different function that is called FROM 'GenerateQuestShip' with those inputs and then it can be called from other place too.
On the other hand, is that not what 'Force_GetShipType' is for?

Levis · Sep 23, 2016

Pieter Boelen said:
What are 'minlevel' and 'maxlevel' there?

Certainly a more reusable function would be good. But for the current 'GenerateQuestShip' functionality, I can't think why those inputs would be needed.
Maybe make a different function that is called FROM 'GenerateQuestShip' with those inputs and then it can be called from other place too.
On the other hand, is that not what 'Force_GetShipType' is for?

Hmmm I forgot to add a character_idx or something like that to the arguments.
min and max level will be used to determined the tier of the ship.
if you want a ship of a specific tier you just say for exampe 4 and 4 so it will always give 4. But if you want for example a pretty good ship but some randomization you could say for example between 3 and 1 so it could generated a tier 3,2 or 1 ship etc.

Pieter Boelen · Sep 23, 2016

Levis said:
min and max level will be used to determined the tier of the ship.
if you want a ship of a specific tier you just say for exampe 4 and 4 so it will always give 4. But if you want for example a pretty good ship but some randomization you could say for example between 3 and 1 so it could generated a tier 3,2 or 1 ship etc.

That is what 'Force_GetShipType' already does . :rolleyes:

Possibly this logic could be moved INTO that function though:

Code:

  if(iNation == PIRATE  && maxclass < MAXPIRATECLASS)  maxclass = MAXPIRATECLASS;
  if(GetCurrentPeriod() <= PERIOD_EARLY_EXPLORERS || GetCurrentPeriod() >= PERIOD_NAPOLEONIC)
  {
  if(iNation == HOLLAND  && maxclass < 3)  maxclass = 3;
  if(iNation == PORTUGAL  && maxclass < 3)  maxclass = 3;
  }

Note that I'm EXCLUDING the Smuggling Coastguard ships here; that will still need to be handled inside 'GenerateQuestShip'.

Changing this could make sense, but has nothing to do with solving the problem here.
In fact, it would make the real problem less visible, because the symptom would be prevented, but the issue would remain.

Levis · Sep 23, 2016

I feel like we're talking about other things...
Force_GetShipType needs min and max levels as input. These need to be determined. i sugest to not calculate them inside the function which generates the ship etc but actually determine it outside of the function on a case by case scenario. Where for example for coastguard ships it just always take a ship between tier 4 and 2 for example while for other cases it might be more determined.

Pieter Boelen · Sep 23, 2016

Both points of my previous post still stand. But indeed for the limitation on Coastguard ships, it could work.
As long as the function doesn't end up having too many inputs, it could indeed give some additional flexibility.

Needs Testing [WIP] Generate Quest Ship Logic Error

Corsair

Attachments

Navigation Officer

Find(Rum) = false;

Navigation Officer

Corsair

Navigation Officer

Find(Rum) = false;

Navigation Officer

Find(Rum) = false;

Navigation Officer

Sea Dog

Attachments

Navigation Officer

Find(Rum) = false;

Navigation Officer

Find(Rum) = false;

Navigation Officer

Find(Rum) = false;

Navigation Officer

Find(Rum) = false;

Navigation Officer