Fixed Shipyard ship replacement- cargo price given as deduction but cargo retained

[pedrwyth]

OK well the skipcargo did n't work because the line to add the cargo has ended up outside the curly brackets of the "if include cargo" statement group in GetSellPrice so the cargo is added whatever the state of the cargoHandler string. (been like that since at least 14 3.4 ) Fixed that and added some explanatory price texts in the buy/sell yes/no box to help show how the prices are comprised. So now re-checking the functionality and chasing after the other problems and will try and pick up this one

Unconfirmed Bug - Shipyard upgrades not factored into sale price | PiratesAhoy!

(which I have confirmed) at the same time. I may yet be some time

 

[Pieter Boelen]

Progress, eh? I do like me some progress!
Nice job, @pedrwyth!

 

[pedrwyth]

Well I was over ambitious in including this one

Confirmed Bug - Ship upgrades interface shows ticks for wrong ship | PiratesAhoy!

which remains unresolved - I will add some detail in that thread.

However here is a shipyard.c where I have fixed the skipcargo/include cargo problems which should mean you get the appropriate inclusion or omission depending if you're selling or swapping and the monies should be correct, removing the "press some buttons for profit" exploit. I have also corrected the upgrades to add value (10% their cost) to a ship that has them and ensured you get that if you remove them (before you got zero for removing them but they were zero value so hey what do you expect?).

I have also added some explanation of how your monies for buying or selling are made up so the ship trade-in value is more explicit (it is also the "ship value" that appears in the interface now rather than the cargo included value as before - which was pretty meaningless if you had a lot of cargo).

So that should have fixed this thread and this other one

Unconfirmed Bug - Shipyard upgrades not factored into sale price | PiratesAhoy!

oops for the second bit of upgrade removal you also need KBroutines

 

[Pieter Boelen]

I have also added some explanation of how your monies for buying or selling are made up so the ship trade-in value is more explicit (it is also the "ship value" that appears in the interface now rather than the cargo included value as before - which was pretty meaningless if you had a lot of cargo).

That sounds really interesting! Is that something you could show in action on a screenshot?

 

[pedrwyth]

That sounds really interesting! Is that something you could show in action on a screenshot?

Like these

 

[Pieter Boelen]

That is really cool, @pedrwyth! More clarity for the players is always good, so I like it a lot.

 

[pedrwyth]

OK if you think more info is good this emphasises the acquired status (which affects price if you have SY_RPG_STYLE on) but otherwise still raises awareness.

I have replaced the previous file I posted to now give this enhanced style.

 

[Pieter Boelen]

Cool!

Didn't I change the text of the Buy button depending on the acquired status?

Anyway, that seems like a very good addition and should raise awareness too.
Nicely done!

 

[pedrwyth]

Didn't I change the text of the Buy button depending on the acquired status?

Yes you did which is partly what made me think of it - only really "extra" info for the trade/swap situation

Anyway, that seems like a very good addition and should raise awareness too.

Yes the only problem is if you get a message about "This officer has too low Leadership and Sailing skills to command this ship." as an additional first line when my addition as a third line will be across the money bag.

 

[Pieter Boelen]

Could that warning be a separate screen?
Or skip the status info only if that warning needs to be displayed at the same time?

It may depend on screen resolution too.

 

[pedrwyth]

Could that warning be a separate screen?
Or skip the status info only if that warning needs to be displayed at the same time?

It may depend on screen resolution too.

On my screen/resolution you get three lines, actually it's self editing because that long competence message takes two lines, the buy ship? is the third (which was already the case if the first message came up) and looks OK. The last just disappears because it can't fit in the text window (wherever that is defined - I haven't looked) - so I'll just leave it as is for now I think.

 

[Pieter Boelen]

Maybe @Levis could give a short clue on how to make the window bigger?
Should be possible, I reckon.

 

[Talisman]

Yes the only problem is if you get a message about "This officer has too low Leadership and Sailing skills to command this ship." as an additional first line when my addition as a third line will be across the money bag.

Could you move the No/Yes buttons further apart ( and/or make them slightly smaller ) - then put the money bag between them ?

 

[Levis]

Maybe @Levis could give a short clue on how to make the window bigger?
Should be possible, I reckon.

You need to change the ini files for the interfaces in the resource folder.
In there the different elements are defined and their locations and size etc.
You can change it in there.

 

[Pieter Boelen]

@pedrwyth about this line of code in KB_routines.c:

  cost = GetCostTun(_idx, _improvement, PRICE_TYPE_BUY)/10; //PW: was PRICE_TYPE_SELL but this returns zero so no refund

Have you got any idea why "sell" returns zero? I don't think it should be doing that.
I'd assume then that this value in GetCostTun returns zero:

  int modifier = GetCharPriceMod(GetMainCharacter(),_priceType,true,true)

But that should not be returning zero either:

float GetCharPriceMod(ref ch, int priceType, bool summ, bool bShipyard)
{
   float skillModify = 1.0;
   float skillDelta;
   if(summ) skillDelta = GetShipSkill  (&ch,SKILL_COMMERCE);
   else skillDelta = CalcCharacterSkill(&ch,SKILL_COMMERCE);
   if(priceType==PRICE_TYPE_BUY)
   {
     skillModify = 1.4 - skillDelta*0.019;
     if(CheckOfficersPerk(&ch,"AdvancedCommerce"))   { skillModify -= 0.2; }
     else
     {
       if(CheckOfficersPerk(&ch,"BasicCommerce"))   { skillModify -= 0.1; }
     }
   }
   else
   {
     bool bCheckSkills = true;
     if ( IsMainCharacter(ch) )
     {
     /*   if (IsInAnyService())    // Levis: Professional Navy and LoM pay commission, but are not affected by skills
       {
         bCheckSkills = false;
         if (bShipyard)     skillModify = SHIPYARD_SHIP_SELL_MULT;                               // skills don't affect shipyard transactions
         else         skillModify = makefloat(((SHIPYARD_PIRATEDSHIP_SELL_MULT+SHIPYARD_BOUGHTSHIP_SELL_MULT)/2));   // 50% commission
       }*/
       if (!bShipyard && HasMerchantPassport(ch))   // PB: Honest merchants don't pay commission outside the shipyard
       {
         bCheckSkills = false;
         skillModify  = 1.0;                                                   // skills don't affect store transactions
       }
     }
     if (bCheckSkills)
     {
       skillModify = 0.75 + skillDelta*0.019;
       if(CheckOfficersPerk(&ch,"AdvancedCommerce"))   skillModify += 0.05;
     }
   }
   return skillModify;
}

So I'm a bit confused....

 

[pedrwyth]

@pedrwyth about this line of code in KB_routines.c:

  cost = GetCostTun(_idx, _improvement, PRICE_TYPE_BUY)/10; //PW: was PRICE_TYPE_SELL but this returns zero so no refund

Have you got any idea why "sell" returns zero? I don't think it should be doing that.

No I was lazy and just changed it to BUY. It doesn't seem to make sense to me either and I wondered if I had been dreaming - but I have just tried it again (admittedly in a Sept based version) and it undoubtedly does have that effect when set as PRICE_TYPE_SELL (and therefore you get no money for removing upgrades)

I'd assume then that this value in GetCostTun returns zero:

  int modifier = GetCharPriceMod(GetMainCharacter(),_priceType,true,true)

But that should not be returning zero either:

float GetCharPriceMod(ref ch, int priceType, bool summ, bool bShipyard)
{
   float skillModify = 1.0;
   float skillDelta;
   if(summ) skillDelta = GetShipSkill  (&ch,SKILL_COMMERCE);
   else skillDelta = CalcCharacterSkill(&ch,SKILL_COMMERCE);
   if(priceType==PRICE_TYPE_BUY)
   {
     skillModify = 1.4 - skillDelta*0.019;
     if(CheckOfficersPerk(&ch,"AdvancedCommerce"))   { skillModify -= 0.2; }
     else
     {
       if(CheckOfficersPerk(&ch,"BasicCommerce"))   { skillModify -= 0.1; }
     }
   }
   else
   {
     bool bCheckSkills = true;
     if ( IsMainCharacter(ch) )
     {
     /*   if (IsInAnyService())    // Levis: Professional Navy and LoM pay commission, but are not affected by skills
       {
         bCheckSkills = false;
         if (bShipyard)     skillModify = SHIPYARD_SHIP_SELL_MULT;                               // skills don't affect shipyard transactions
         else         skillModify = makefloat(((SHIPYARD_PIRATEDSHIP_SELL_MULT+SHIPYARD_BOUGHTSHIP_SELL_MULT)/2));   // 50% commission
       }*/
       if (!bShipyard && HasMerchantPassport(ch))   // PB: Honest merchants don't pay commission outside the shipyard
       {
         bCheckSkills = false;
         skillModify  = 1.0;                                                   // skills don't affect store transactions
       }
     }
     if (bCheckSkills)
     {
       skillModify = 0.75 + skillDelta*0.019;
       if(CheckOfficersPerk(&ch,"AdvancedCommerce"))   skillModify += 0.05;
     }
   }
   return skillModify;
}

So I'm a bit confused....

I'm not currently doing anything (I've left the value for cargo on freeplay starts to take a break since I found the repair costs were n't right either - no rush on that one anyway.) - so I'll look at the code a bit further to see if I can spot what's happening

 

[Pieter Boelen]

Thanks, @pedrwyth! This is probably not so important, but I cannot help being curious.

 

[pedrwyth]

OK the key is in the

int modifier = GetCharPriceMod(GetMainCharacter(),_priceType,true,true)

whereas

float GetCharPriceMod(ref ch, int priceType, bool summ, bool bShipyard)

so the deeper function returns a float which for sell is less than 1 and for buy over 1. The int of the modifier truncates them to 0 and 1 respectively so you get 0 on sell and 1 on buy so change it to

float modifier = GetCharPriceMod(GetMainCharacter(),_priceType,true,true)

and all works as intended and you can put the SELL back in selling

Incidentally the float in

int GetCostTun(int _idx, string _improvement, float _priceType)

is none too clever either since it is either a 0 or 1 define (and later passed down as an int).

 

[Pieter Boelen]

the deeper function returns a float which for sell is less than 1 and for buy over 1. The int of the modifier truncates them to 0 and 1 respectively so you get 0 on sell and 1 on buy

Ahaaa! That does explain a WHOLE lot. Sounds like a very good fix to make indeed.
Thanks for figuring it out.