UI: Show when the max number of servers have been bought in stores.

[tomprince]

[d0sboots]

This shouldn't be a brand-new button; it should change the properties of the existing button (and thus, also be disabled).

[tomprince]

This shouldn't be a brand-new button; it should change the properties of the existing button (and thus, also be disabled).

I followed exactly the pattern for the cores button on the same page: https://github.com/bitburner-official/bitburner-src/blob/dev/src%2FLocations%2Fui%2FCoresButton.tsx#L18

[d0sboots]

You're misunderstanding my complaint. There is already a button for buying a server (obviously, since it is possible to do.) What you did creates a different (new) button, and returns that in the case that max servers have been reached. I am saying that instead, you should modify the existing button so that it conditionally has the text you want to add.

[tomprince]

You're misunderstanding my complaint.

I don't think I am. I'm pointing out that the existing code for the same behavior of the cores button is the same.

What you did creates a different (new) button,

This isn't really how react works; it doesn't differentiate a button by which but of code created it. With either way of writing the code, react should match up the button to the same HTML element.

[d0sboots]

This isn't really how react works; it doesn't differentiate a button by which but of code created it. With either way of writing the code, react should match up the button to the same HTML element.

What React is doing is irrelevant. This is entirely about code structure and maintainability, not React. It is bad practice to duplicate code like this, especially when the change you need to make is only to a single property. It makes maintenance harder and increases the chance of bugs (which already happened - the new button is not properly disabled.)

[tomprince]

This is entirely about code structure and maintainability, not React.

Every other button on the page uses the same style, so it seems like needless variation to make this one button different.

Also, I think it makes sense to be styled differently when maxed out vs. not having enough money. The current buttons have that by not being disabled, which is a reasonable style choice. However, I've gone and created a new button variant, and used the "secondary" colour (which in the default theme is a dull grey) for the text instead.

Also, is something like this

  const isMaxed = Player.purchasedServers.length >= getPurchaseServerLimit();
  return (
    <>
      <Button onClick={() => setOpen(true)} disabled={!Player.canAfford(cost) || isMaxed} variant={isMaxed ? "maxed" : undefined}>
        Purchase {formatRam(props.ram)} Server&nbsp;-&nbsp;
        {isMaxed ? "MAX SERVERS" : <Money money={cost} forPurchase={true} />}
      </Button>
      <PurchaseServerModal open={open} onClose={() => setOpen(false)} ram={props.ram} cost={cost} />
    </>
  );

really clearer than

  if (Player.purchasedServers.length >= getPurchaseServerLimit()) {
    return (
      <Button disabled={true} variant="maxed">Purchase {formatRam(props.ram)} Server&nbsp;-&nbsp;MAX SERVERS</Button>
    );
  }
  return (
    <>
      <Button onClick={() => setOpen(true)} disabled={!Player.canAfford(cost)}>
        Purchase {formatRam(props.ram)} Server&nbsp;-&nbsp;
        <Money money={cost} forPurchase={true} />
      </Button>
      <PurchaseServerModal open={open} onClose={() => setOpen(false)} ram={props.ram} cost={cost} />
    </>
  );

In particular, can you tell at a glance what is different when isMaxed is true in the first case?

[d0sboots]

This is entirely about code structure and maintainability, not React.

Every other button on the page uses the same style, so it seems like needless variation to make this one button different.

They shouldn't, although I can understand from a code evolution standpoint why they started out that way. The second example clearly shows the code rot around that pattern though - the "maxed" button doesn't have the tooltip, nor is it disabled, not does it have the explanatory text around the button. None of that stuff should change just because you max out the RAM.

Fixing those is out of scope for this PR, of course. Unless you wanted to.

Also, is something like this

  const isMaxed = Player.purchasedServers.length >= getPurchaseServerLimit();
  return (
    <>
      <Button onClick={() => setOpen(true)} disabled={!Player.canAfford(cost) || isMaxed} variant={isMaxed ? "maxed" : undefined}>
        Purchase {formatRam(props.ram)} Server&nbsp;-&nbsp;
        {isMaxed ? "MAX SERVERS" : <Money money={cost} forPurchase={true} />}
      </Button>
      <PurchaseServerModal open={open} onClose={() => setOpen(false)} ram={props.ram} cost={cost} />
    </>
  );

really clearer than

  if (Player.purchasedServers.length >= getPurchaseServerLimit()) {
    return (
      <Button disabled={true} variant="maxed">Purchase {formatRam(props.ram)} Server&nbsp;-&nbsp;MAX SERVERS</Button>
    );
  }
  return (
    <>
      <Button onClick={() => setOpen(true)} disabled={!Player.canAfford(cost)}>
        Purchase {formatRam(props.ram)} Server&nbsp;-&nbsp;
        <Money money={cost} forPurchase={true} />
      </Button>
      <PurchaseServerModal open={open} onClose={() => setOpen(false)} ram={props.ram} cost={cost} />
    </>
  );

In particular, can you tell at a glance what is different when isMaxed is true in the first case?

Yes. In particular, I can not easily tell what is different between the two in the second case, and especially I can't tell if those differences are intentional, or just bitrot because someone didn't update both branches.

[d0sboots]

So, I'm not completely against this, but... why are maxed things special in this way? What makes a maxed button unique, such that it should have a different visual affordance beyond simply being disabled?

If we are going to do this, I think it implies a larger effort that might either be outside the scope of this PR, or greatly expand the scope of this PR, because there are a lot of things you can max in this game, and presumably they all would deserve the same treatment?

[tomprince]

The difference is you can eventually (by getting enough money) go from disabled to not, but nothing[1] will cause a button to go from maxed to not. I think making this easy to see at a glance is worthwhile.

[1] There are exceptions, such as NS.deleteServer.

[d0sboots]

Well, installing augmentations will cause these buttons to stop being maxed, but I could see considering that in a different "category" of actions.

But I'm also interested in the 2nd part. By that logic, it seems like this styling should apply to almost all maxable buttons - upgrades in hashnet, corp, etc. I'm on the fence about whether styling these differently from disabled is helpful, but I think styling them differently from all the other things that we can max doesn't seem to make sense.

[tomprince]

So, the reason I went with a distinct look (at least from disabled, if not also from not-disabled), is that that was the existing behaviour of the cores and ram buttons.

When I had maxed out cores and not ram, it would show

• the core button as all green
• the ram button
  • green with yellow money if I could afford something
  • turquoise if I couldn't

That makes it easy to see at a glance what I could buy, what I could eventually afford, and what I had already maxed out.

That would probably make sense for other things, but I've not run into them often enough to notice the behaviour one way or another. Except for the servers, which is what prompted this PR.

[d0sboots]

I just noticed that the same issue applies to the "TOR router" button - it also appears as interactable when the TOR router is purchased, probably for the same underlying reason.

What about this as an option?

• Able to upgrade and have enough money: Green with yellow money, same as now
• Unable to upgrade due to not enough money: All turquoise, same as now
• Unable to upgrade due to all upgrades purchased: Turquoise label, quantity ("MAX" or "MAX SERVERS") in green (or maybe some other color, but green/primary was the most obvious).

This would satisfy the desire to have the three options be visually distinct. It can also be implemented without needing a new variant for the Button. And it makes sense situationally; the button is still disabled, but with an additional flair to indicate that it's different from "normal" disabled. This might, or might not, make sense to apply to other buttons.