Fighter II checkbox #3

chelseaSchmidt · 2024-07-31T00:10:00Z

Took a stab at this, check my logic though!

New budget inputs:

shipCapacityUsed
maxShipCapacity
spaceDockFighterBonus

New budget display field:

shipCapacityRemaining: maxShipCapacity + spaceDockFighterBonus - shipCapacityUsed - fighters added with the calculator
- Fighter I: All fighters count against shipCapacityRemaining
- Fighter II: Fighters count against shipCapacityRemaining until it reaches 0, then they begin to count against fleetSupplyRemaining

I realized there should also be a checkbox for "I'm building in a space-only system" for Saar (maybe others?), which would trigger ground forces to be subtracted from shipCapacityRemaining, but leaving that for another PR!

chelseaSchmidt · 2024-07-31T00:16:28Z

frontend/src/Calculator.tsx

+                        <Checkbox
+                            label="Fighter II"
+                            checked={isFighterUpgraded}
+                            onChange={() =>
+                                setIsFighterUpgraded(!isFighterUpgraded)
+                            }


I was on the fence about putting the checkbox inside BudgetModeInputs rather than adjacent, but I could foresee wanting to use the checkbox in both modes if we get fancier, and putting it here makes it easier to move around if that happens. Not married to it either way though!

I think I lean (slightly) the other way, but that's because I only really imagine using this in the budget mode. Doesn't really make a difference to me as this is easy to refactor if we guess wrong.

💭 One thing I am considering though - I think with all this functionality, budget mode is getting a little complex. I like the functionality, but it's a lot of paperwork to do. What would you think about (in the future, not this PR) hiding some of the more complex options like capacity management or off-planet (read: Saar) production, under a dropdown tab for "advanced mode"? That way the options are there for anyone who wants to deal with them, and hidden (+inactive) for anyone who can't be bothered?

Another example in support of an "advanced mode" - isn't the "Space Dock Fighter Bonus" the same value for almost every faction?

Done, moved firmly into budget mode 👌

That's a great idea, all the form fields are making my eyes glaze over!

Oh dang didn't know that about the space dock bonus - the only two factions I've drafted have been different so I just assumed it varied! Should definitely make that a static number which you can change in advanced mode

sirrus233

Solid stuff!!! Looks fantastic 😍

sirrus233 · 2024-07-31T05:55:20Z

frontend/src/data.ts

@@ -87,20 +87,36 @@ export function getTotalCost(unitCounts: UnitCounts): number {
    );
 }

+function sumUnitCountEntries(unitCountEntries: [Unit, number][]): number {


The weirdness of dealing directly with a [Unit, number][] type struck me. Try playing around with this function signature and see if you like what falls out!

function sumUnitCounts( unitCounts: UnitCounts, unitFilter: (unit: Unit) => boolean = () => true): number )

Sooo much cleaner, thank you, updated! I love this 🔥

sirrus233 · 2024-07-31T05:57:11Z

frontend/src/data.ts

 }

 export function getFleetSupplyRemaining(
    currentFleetSupply: number,
    maxFleetSupply: number,
-    unitCounts: UnitCounts
+    unitCounts: UnitCounts,
+    airborneFighterCount: number


⛏️ Took just a sec to realize what this name meant. Maybe unsupportedFighters? What do you think?

That's perfect, I was so stuck on this name and could only think of nonTransportedFighters but didn't like the negation. And airborne isn't even correct because they're in space? Lol, fixed now

sirrus233 · 2024-07-31T06:02:35Z

frontend/src/Calculator.tsx

+                        <Checkbox
+                            label="Fighter II"
+                            checked={isFighterUpgraded}
+                            onChange={() =>
+                                setIsFighterUpgraded(!isFighterUpgraded)
+                            }


I think I lean (slightly) the other way, but that's because I only really imagine using this in the budget mode. Doesn't really make a difference to me as this is easy to refactor if we guess wrong.

💭 One thing I am considering though - I think with all this functionality, budget mode is getting a little complex. I like the functionality, but it's a lot of paperwork to do. What would you think about (in the future, not this PR) hiding some of the more complex options like capacity management or off-planet (read: Saar) production, under a dropdown tab for "advanced mode"? That way the options are there for anyone who wants to deal with them, and hidden (+inactive) for anyone who can't be bothered?

sirrus233 · 2024-07-31T06:15:39Z

frontend/src/Calculator.tsx

+                        <Checkbox
+                            label="Fighter II"
+                            checked={isFighterUpgraded}
+                            onChange={() =>
+                                setIsFighterUpgraded(!isFighterUpgraded)
+                            }


Another example in support of an "advanced mode" - isn't the "Space Dock Fighter Bonus" the same value for almost every faction?

sirrus233 · 2024-07-31T06:29:22Z

frontend/src/BudgetModeDisplay.tsx

 }: Props) {
+    const shipCapacityRemaining =


⛏️ This variable name kinda bothers me, only because elsewhere you're careful to distinguish between "ship capacity" and the "space dock fighter bonus" (I'm guessing to later account for ground forces, which can't be supported by space docks?). But here it's all lumped together and called "ship capacity".

A few alternate proposals:
totalCapacityRemaining
capacityRemaining
fighterCapacityRemaining (I think maybe I dislike this one the least)

Right guess on the future ground forces problem, and yes good point - I like fighterCapacityRemaining since that's the least ambiguous, and can handle renaming it when we add advanced mode!

sirrus233 · 2024-07-31T06:31:57Z

frontend/src/BudgetModeDisplay.tsx

            value: capacityBudget - getTotalCapacity(unitCounts),
        },
        {
            label: "Fleet Supply Remaining",
            value: getFleetSupplyRemaining(
                currentFleetSupply,
                maxFleetSupply,
-                unitCounts
+                unitCounts,
+                isFighterUpgraded ? Math.max(0 - shipCapacityRemaining, 0) : 0


Had to puzzle over this, and see it work in action, before I could convince myself it was correct (it is!). Probably just sleep-deprived ☕

But consider:
isFighterUpgraded && shipCapacityRemaining < 0 ? shipCapacityRemaining : 0

Neither is wrong, so pick your fave.

Honestly same, and I'm the one that wrote it 🤦‍♀️ yours is definitely easier on the brain! Updated to that with a teeny adjustment to flip the sign

sirrus233 · 2024-07-31T06:39:55Z

frontend/src/BudgetModeDisplay.tsx

            ),
        },
+        {
+            label: "Ship Capacity Remaining",
+            value: isFighterUpgraded


As above, check if you prefer:
isFighterUpgraded && shipCapacityRemaining < 0 ? 0 : shipCapacityRemaining

Prefer yours, updated! I was playing around with a zeroFloor(num: number) { return Math.max(num, 0); } util to abstract out the Math.max(x, 0)s everywhere but it still ended up being hard to digest, I give up 🥲

Oooh and, did one more thing, the latest commit reduces the confusion even more - 47d71a6

chelseaSchmidt · 2024-08-01T07:52:35Z

@sirrus233 Aaah been out all day and just had time to check this out, these review comments are so on point, dude thank you!! Gonna see if I can get a jump on these before we call tomorrow!

chelseaSchmidt · 2024-08-01T22:34:44Z

@sirrus233 I think I got everything, you want to take one more look?

Each time I change any of the math I wish for unit tests, and yet am too lazy to set up Jest. I will simply complain about it endlessly

sirrus233 · 2024-08-01T22:53:07Z

#6

sirrus233

🚢

sirrus233 · 2024-08-01T23:03:40Z

frontend/src/data.ts

@@ -87,20 +87,34 @@ export function getTotalCost(unitCounts: UnitCounts): number {
    );
 }

-export function getTotalCapacity(unitCounts: UnitCounts): number {


Nice catch! I missed the equivalency.

chelseaSchmidt added the enhancement New feature or request label Jul 31, 2024

chelseaSchmidt requested a review from sirrus233 July 31, 2024 00:10

chelseaSchmidt commented Jul 31, 2024

View reviewed changes

sirrus233 approved these changes Jul 31, 2024

View reviewed changes

Fighter II checkbox

bac7863

chelseaSchmidt force-pushed the fighter-checkbox branch from a5fa6c6 to bac7863 Compare August 1, 2024 20:38

chelseaSchmidt added 7 commits August 1, 2024 13:46

🏗️ Move checkbox inside BudgetModeInputs

5f34f0f

♻️ sumUnitCountEntries (blegh) -> sumUnitCounts

d4b2aec

🎨 airborne(spaceborn?)Fighters -> unsupportedFighters

b0002c1

🎨 shipCapacityRemaining -> fighterCapacityRemaining

3b80e2e

🎨 Fix confusing unsupported fighter calculation

f690af0

🎨 Fix confusing fighter-capacity-remaining display value calc

9cafc7b

🎨 clarify even more, fighterCapacityRemaining calcs

47d71a6

sirrus233 approved these changes Aug 1, 2024

View reviewed changes

chelseaSchmidt merged commit d5b19e1 into main Aug 1, 2024

chelseaSchmidt deleted the fighter-checkbox branch August 1, 2024 23:59

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Fighter II checkbox #3

Fighter II checkbox #3

chelseaSchmidt commented Jul 31, 2024

chelseaSchmidt Jul 31, 2024

sirrus233 Jul 31, 2024

sirrus233 Jul 31, 2024

chelseaSchmidt Aug 1, 2024

chelseaSchmidt Aug 1, 2024 •

edited

Loading

sirrus233 left a comment

sirrus233 Jul 31, 2024

chelseaSchmidt Aug 1, 2024

sirrus233 Jul 31, 2024

chelseaSchmidt Aug 1, 2024

sirrus233 Jul 31, 2024

sirrus233 Jul 31, 2024

sirrus233 Jul 31, 2024

chelseaSchmidt Aug 1, 2024

sirrus233 Jul 31, 2024

chelseaSchmidt Aug 1, 2024

sirrus233 Jul 31, 2024

chelseaSchmidt Aug 1, 2024

chelseaSchmidt Aug 1, 2024

chelseaSchmidt commented Aug 1, 2024

chelseaSchmidt commented Aug 1, 2024

sirrus233 commented Aug 1, 2024

sirrus233 left a comment

sirrus233 Aug 1, 2024

chelseaSchmidt Aug 1, 2024

Fighter II checkbox #3

Fighter II checkbox #3

Conversation

chelseaSchmidt commented Jul 31, 2024

New budget inputs:

New budget display field:

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

chelseaSchmidt Aug 1, 2024 • edited Loading

Choose a reason for hiding this comment

sirrus233 left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

chelseaSchmidt commented Aug 1, 2024

chelseaSchmidt commented Aug 1, 2024

sirrus233 commented Aug 1, 2024

sirrus233 left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

chelseaSchmidt Aug 1, 2024 •

edited

Loading