• 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!

Fixed Remove unnecessary code duplication in SBGetShoreLeavePay

jsv

jsv

Freebooter
Storm Modder
SBGetShoreLeavePay calculates (reduced) salaries all by itself, while it should use the common routines. Before addressing that I want to clean up officers' payments first.
 
jsv said:
SBGetShoreLeavePay calculates (reduced) salaries all by itself, while it should use the common routines.
Click to expand...
Indeed I HATE unnecessary code duplication. That's just asking for trouble.
So if you can address that too, it'll be much appreciated.

I'm changing this to "Feature Request", otherwise it may end up being archived prematurely. :facepalm
 
Pieter Boelen said:
So if you can address that too, it'll be much appreciated.
Click to expand...
I plan to, but I'll need to use the functions that calculate officer salaries, so first I want to be sure those are correct.
There are recent complains about officer salaries being too low. And that BBF story makes me wonder if what we actually pay always matches what is shown on the character screen.
 
I think if you convince all code related to officer salaries to use the SAME functionality, then at least there will be internal consistency between what you see everywhere.
That may make it easier for @Levis to then balance the numbers we actually get.
 
Well, I'm getting somewhere with that officers' stuff. I hope to finish fixing this one (together with crew hire prices) tomorrow.
By the way, this particular piece of code didn't seem to apply SALARY_MULTIPLIER... All my fixes so far are making things more expensive, mwa-ha-ha! :pirate07:
 
jsv said:
Well, I'm getting somewhere with that officers' stuff. I hope to finish fixing this one (together with crew hire prices) tomorrow.
Click to expand...
Good news! Thanks again. :cheers

jsv said:
By the way, this particular piece of code didn't seem to apply SALARY_MULTIPLIER...
Click to expand...
I'm not sure if you reasonably can apply that multiplier to officer prices.
Or that is to say: I'd imagine the combined officer pay to be equal to the sum of the "monthly salaries" shown in F2>Character.

So the multiplier would have to be included at the point where the salaries are determined AND updated, which @Levis' recently made changes to.
Alternatively, the multiplier would have to be applied after that fact everywhere you get to see the salary.

jsv said:
All my fixes so far are making things more expensive, mwa-ha-ha! :pirate07:
Click to expand...
Ha! Well, we wouldn't want the game to be too easy, would we? People have been complaining before that they make money too quickly.
And since you're adding SALARY_MULTIPLIER everywhere, if it becomes truly too much, we can decrease that default value.
 
Pieter Boelen said:
I'm not sure if you reasonably can apply that multiplier to officer prices.
Click to expand...
I meant it wasn't applied when paying the crew on shore leave.

It is applied to all officer prices (at least all that I have checked so far). It was so before my fixes and will remain so.
 
If you need any help let me know. else I will focus on other stuff
 
Hmm... In the berthing code:
Code: 
// ***** 1: WORK OUT PAY FOR CREW *****
    float amCrewPaymentQ = 5 + amCrewQty*12
    amCrewPaymentQ *= (1.0 - (makefloat(amLeaderShip)*makefloat(1.0+GetOfficersPerkUsing(amCaptainChar,"IronWill"))/40.0)) * (0.5 + makefloat(GetDifficulty())*0.5);
    if (TRACELOG == 1) { Trace("shore leave pay calculation: raw crew pay " + amCrewPaymentQ); }

    // ***** 2: REDUCE CREW PAY *****
    if (amCrewPaymentQ > 0) { amCrewPaymentQ /= SHORE_LEAVE_PAY_REDUCTION; }
OK, 12 is BASE_CREW_PAY, and the multiplier in the next string matches what we use all the time. But why that 5 is added in the very beginning? Does it represent a nominal berthing fee that you pay even if you do not happen to have any crew?

P.S. I haven't got much chance to work on it today and the code utilizing the above is a true beauty:
Code: 
    int tempnumz1;
    int tempnumz2;
    string tempstringz1;
    string tempstringz2;
    int tempnumz3;
    int tempnumz4;
    int tempnumz5;
    int tempnumz6;

    for(tempnumz1=0; tempnumz1<TOWNS_QUANTITY; tempnumz1++) // screwface
    {
        tempstringz1 = "port" + tempnumz1;
        if(!CheckAttribute(zMainChar,"ShipBerthing."+tempstringz1)) continue; // screwface

        for (tempnumz2=1; tempnumz2<5; tempnumz2++)
        {
            tempstringz2 = "slot" + tempnumz2;
            if(!CheckAttribute(zMainChar,"ShipBerthing." + tempstringz1 + "." + tempstringz2)) continue; // screwface

            tempnumz3 = zMainChar.ShipBerthing.(tempstringz1).(tempstringz2).status;
            if (tempnumz3 > 0)
            {
                tempnumz4 = sti(zMainChar.ShipBerthing.(tempstringz1).(tempstringz2).nowdue);
                tempnumz5 = sti(zMainChar.ShipBerthing.(tempstringz1).(tempstringz2).dailycost);
                tempnumz4 += tempnumz5;
                zMainChar.ShipBerthing.(tempstringz1).(tempstringz2).nowdue = tempnumz4;
                tempnumz4 = sti(zMainChar.ShipBerthing.(tempstringz1).(tempstringz2).daysinberth);
                tempnumz4 += 1;
                zMainChar.ShipBerthing.(tempstringz1).(tempstringz2).daysinberth = tempnumz4;
            }
            if (tempnumz3 == 2 && sti(worldMap.date.day) == 7)
            {
                tempnumz4 = sti(zMainChar.ShipBerthing.(tempstringz1).(tempstringz2).crewnowdue);
                tempnumz5 = sti(zMainChar.ShipBerthing.(tempstringz1).(tempstringz2).captainindex);
                tempnumz6 = GetCrewQuantity(GetCharacter(tempnumz5));
                tempnumz4 += SBGetShoreLeavePay(tempnumz5,tempnumz6);
                zMainChar.ShipBerthing.(tempstringz1).(tempstringz2).crewnowdue = tempnumz4;
            }
        }
    }
:rolleyes: ... not breaking THAT is going to take some extra time, I guess.
 
jsv said:
But why that 5 is added in the very beginning? Does it represent a nominal berthing fee that you pay even if you do not happen to have any crew?
Click to expand...
That's the only explanation that I could think of.
But for berthing just the hull, I imagine that should not be included in the "crew costs"?
There are two different types of berthing anyway: Laying Up (stripped down bare hull) and Shore Leave (full ship + officers).
 
I'm removing that 5 from the crew pay calculation, then. If it will turn out one can berth empty ships for free as the result, we'll rectify it elsewhere.

And this is really nice:
Code: 
    int SHORE_LEAVE_PAY_REDUCTION = 4;
    ...
    // ***** 3: WORK OUT OWN PAY AND OFFICERS' PAY *****
 
    ...    
    for(tempnumam1 = 1; tempnumam1 < 4; tempnumam1++)
    {
        tempnumam2 = GetOfficersIndex(amCaptainChar, tempnumam1);
        if(tempnumam2 == -1) continue;
        if(CheckAttribute(Characters[tempnumam2],"quest.OfficerPrice") && GetRemovable(&Characters[tempnumam2]))
        {
            tempfloatam1 = CalcEncOfficerPrice(Characters[tempnumam2])/5;        // LDH 16Apr09
            if (TRACELOG == 1) { Trace("shore leave pay calculation: officer taken into account, idx " + tempnumam2 + ", pay " + tempfloatam1); }
            amOfficerPaymentQ += tempfloatam1;
        }
    }
  
...
    if (amOfficerPaymentQ > 0) { amOfficerPaymentQ /= SHORE_LEAVE_PAY_REDUCTION; }
CalcEncOfficerPrice returns the monthly officer's salary as shown on the interface screen. So, while crew salary on shore leave is quartered, officers' payments are reduced by a factor of 20(!).
That's very ... progressive... of us, I think.

whores and hounds n' navy rum
they have me broke, they have me numb
a drunken sailor i become whenever i'm on the shore
:wp
 
Last edited:
jsv said:
CalcEncOfficerPrice returns the monthly officer's salary as shown on the interface screen. So, while crew salary on shore leave is quartered, officers' payments are reduced by a factor of 20(!).
Click to expand...
Where is that done? I see a factor of 5 there. Or is that crew factor of 4 applied to officers as well so that it becomes 4*5=20?
 
jsv said:
Yep, in the last line of the quoted part.
Click to expand...
Sounds to me like it should be "total crew pay + total officer pay) * SHORE_LEAVE_PAY_REDUCTION" or something like that.
No need for separate factors cluttered everywhere through the code, is there?
 
I agree. Unless some players have so numerous crew and such big staff that adding their salaries together is going to cause an fp overflow, I see no reason not to simplify it like that :)
 
Partially resolved here: Crew hiring cost.

The code now uses common function for difficulty and leadership adjustments, crew and officers' salary on the leave are calculated more uniformly.
It would be nice to factor out the "loop across captains and their subordinates and collect officerprices" part. It's done in many places in the code, each time a bit differently. But that would be a separate project.

I've tested the changes I've made to berthing only briefly, but they seem to work and it was pretty much broken anyway.
Crew+officers' payment on shore leave is now 1/4 of their normal salary.

There is an unrelated problem with berthing charges: "Note however that berthing charges will be high (0 / month) and you will continue to pay the crew...". Without my changes it's still 0/month, so that's a separate issue.
 
I just installed your changes into my game. Exactly what I had hoped you would do; that definitely clears things out quite nicely!
Thank you very much. :cheers

jsv said:
There is an unrelated problem with berthing charges: "Note however that berthing charges will be high (0 / month) and you will continue to pay the crew...". Without my changes it's still 0/month, so that's a separate issue.
Click to expand...
I had a quick look at that and it seems to be technically "Not a Bug".
The relevant function is SBCalculateDailyCost in PROGRAM\INTERFACE\kam_shipberthing_ship.c .

There was this code in place:
Code: 
switch (sti(ShipsTypes[STNum].class))
   {
     case 8:     ShipCostBracket = 0;     break;
     case 7:     ShipCostBracket = 0;     break;             //   bracket 0 is for boats (class 7) - in this bracket berthing is free
     case 6:     ShipCostBracket = 1;     break;             //   bracket 1 is for small to medium ships (classes 6 to 4)
     case 5:     ShipCostBracket = 1;     break;             //   bracket 2 is for capital ships (classes 3 and 2)
     case 4:     ShipCostBracket = 1;     break;             //   bracket 3 is for battleships
     case 3:     ShipCostBracket = 2;     break;             //   bracket 4 is for manowars
     case 2:     ShipCostBracket = 2;     break;
     case 1:
       switch(ShipsTypes[STNum].name)
       {
         case "Manowar1":   ShipCostBracket = 4;     break;
         case "Manowar2":   ShipCostBracket = 4;     break;
         case "Manowar_gub":   ShipCostBracket = 4;     break;
         //default:    
           ShipCostBracket = 3;     break;
       }
     break;
   }
So laying up small ships could be free because of that '0' value there.
And this section could also drop the costs, theoretically even below zero:
Code: 
  switch (PortCostBracket)
   {
     case 0:     tempcoefficient -= 0.5;     break;
     case 2:     tempcoefficient += 0.5;     break;
   }

I am replacing the "ship class" check with a straight formula as it doesn't make sense to single out 3 out of the 200 ships:
Code: 
  // STEP 1/2: SET PRICING BRACKETS

   int ShipCostBracket = (9-sti(ShipsTypes[STNum].class))/2;
   if (TRACELOG == 1) { trace("ship placed in ship cost bracket " + ShipCostBracket); }
You'd be welcome to have a look at the rest of that code and simplify it a bit further.
It looks fairly straightforward.

In any case, when I gave myself a big ship, I got to see higher berthing costs than '0'. :yes
 
  • Like
Reactions: jsv
@jsv: I think this is technically "Fixed" now, right? Or is there anything more to be done?
 
You must log in or register to reply here.
Back
Top