fishpondstudio/CivIdle

Allow Setting a "Full" Percentage for Warehouse Autopilot Mode

Closed this issue · 20 comments

So instead of using "full" building, allow setting a percentage, say 95%. Autopilot mode will transport resources from buildings that has storage percentage >= 95%.

Ref: https://discord.com/channels/631551126377857044/1107770418116448417/1205562241655775322

Should this be a gameplay setting? Having this as an individual setting on each warehouse would mean that each warehouse needs to check every building's storage to see if it's storage is above a certain percentage every tick.

As I understand it, getting building storage is not the most efficient thing to do .

Would it make sense to store something in Tick.next in tickTile, (which already gets the storage) where buildings storage is above a certain percentage?

The current warehouse autopilot mode code is here:
https://github.com/fishpondstudio/CivIdle/blob/d0b063b9a1764d1051479e2c0bbad8e4bf48a2c4/shared/logic/Update.ts#L416C23-L416C32
It's mostly a duplicate of the main transport logic with some modifications (and there are some already known edge cases).

That only gets buildings from notProducingReasons where the reason is StorageFull though. I assume you don't want to be changing the code that affects StorageFull, otherwise that affects the Well Stocked Happiness mechanic.

And you are right, implementing this logic might have a severe negative impact on performance.
Implementing a global "full" threshold maybe better - it will unify all the "full" logic

https://github.com/fishpondstudio/CivIdle/blob/d0b063b9a1764d1051479e2c0bbad8e4bf48a2c4/shared/logic/Update.ts#L347C10-L347C20

That only gets buildings from notProducingReasons where the reason is StorageFull though. I assume you don't want to be changing the code that affects StorageFull, otherwise that affects the Well Stocked Happiness mechanic.

You are right, it will affect well-stocked happiness, but it should not cause a big issue?

This is a very fundamental change to the logic so I am not sure what's the impact as a whole - might need to experiment and see.

Seems to work quite well. I'll get it committed and you can decide for yourself.

image
image

@insraq Does the autopilot always pull from the same building until it is no longer considered full?

Looks like having a percentage will pull from the same building until the storage is below the percentage threshold, which is 95%

cd3f190422e9f13936d7b919a2f63db3.mp4

@insraq Does the autopilot always pull from the same building until it is no longer considered full?

Looks like having a percentage will pull from the same building until the storage is below the percentage threshold, which is 95%

cd3f190422e9f13936d7b919a2f63db3.mp4

In the PR, all buildings with storage >= 95% are added to a list. A warehouse sorts the list by distance and pulls resources from the closest one. So the effect is that a warehouse will pull from the closet building until it's no longer on the list (i.e. < 95%)

I agree this behavior is a bit confusing. And the more I look at it, the more I think the root cause is not related to warehouse autopilot. Rather, it's the "storage full" logic. Currently storage full logic does not cover some edge cases, which causes problems. Maybe it's better to allow a player simply set 95% storage = full and apply all the storage full logic. This simplifies the logic quite a bit.

@insraq Does the autopilot always pull from the same building until it is no longer considered full?
Looks like having a percentage will pull from the same building until the storage is below the percentage threshold, which is 95%
cd3f190422e9f13936d7b919a2f63db3.mp4

In the PR, all buildings with storage >= 95% are added to a list. A warehouse sorts the list by distance and pulls resources from the closest one. So the effect is that a warehouse will pull from the closet building until it's no longer on the list (i.e. < 95%)

I agree this behavior is a bit confusing. And the more I look at it, the more I think the root cause is not related to warehouse autopilot. Rather, it's the "storage full" logic. Currently storage full logic does not cover some edge cases, which causes problems. Maybe it's better to allow a player simply set 95% storage = full and apply all the storage full logic. This simplifies the logic quite a bit.

Allowing the player to set 95% = Full Storage just sounds like moving the problem to somewhere else, if the player has it set at 99% and then changes it to 95% then the same problem will happen.

@insraq Does the autopilot always pull from the same building until it is no longer considered full?
Looks like having a percentage will pull from the same building until the storage is below the percentage threshold, which is 95%
cd3f190422e9f13936d7b919a2f63db3.mp4

In the PR, all buildings with storage >= 95% are added to a list. A warehouse sorts the list by distance and pulls resources from the closest one. So the effect is that a warehouse will pull from the closet building until it's no longer on the list (i.e. < 95%)
I agree this behavior is a bit confusing. And the more I look at it, the more I think the root cause is not related to warehouse autopilot. Rather, it's the "storage full" logic. Currently storage full logic does not cover some edge cases, which causes problems. Maybe it's better to allow a player simply set 95% storage = full and apply all the storage full logic. This simplifies the logic quite a bit.

Allowing the player to set 95% = Full Storage just sounds like moving the problem to somewhere else, if the player has it set at 99% and then changes it to 95% then the same problem will happen.

Yes, but at least it is consistent. If 95% is considered full, then warehouse will drain a building until it is considered not full.
I think the first step is to review the building full logic - and see if we can improve the calculation without asking players to set a threshold value.

Do you want to hold off on the PR for now?

I think this line is potentially bad:

const hasEnoughStorage = isEmpty(output) || used + getStorageRequired(output) <= total;

It probably be more aggressive like: used + getStorageRequired(input * stockpileCapacity)+ getStorageRequired(output) <= total Have you tried to figure out a better logic?

(w.r.t this P.R. we can hold off for now. If you figure out why is that line bad, then feel free to open a new PR, if that works well, then we no longer need this one. Otherwise we might have to revisit this one)

Will take a look 👍

I think this line is potentially bad:

const hasEnoughStorage = isEmpty(output) || used + getStorageRequired(output) <= total;

It probably be more aggressive like: used + getStorageRequired(input * stockpileCapacity)+ getStorageRequired(output) <= total Have you tried to figure out a better logic?
(w.r.t this P.R. we can hold off for now. If you figure out why is that line bad, then feel free to open a new PR, if that works well, then we no longer need this one. Otherwise we might have to revisit this one)

Is hasEnoughStorage used for anything else or is it just to check for the StorageFull and add it to the notProducingReasons.

const hasEnoughStorage = isEmpty(output) || used + getStorageRequired(output) <= total * 0.99;

It's a cheap solution and may cause more problems or edge cases? Depends on if the check is used more than just notProducingReasons.

I think this line is potentially bad:

const hasEnoughStorage = isEmpty(output) || used + getStorageRequired(output) <= total;

It probably be more aggressive like: used + getStorageRequired(input * stockpileCapacity)+ getStorageRequired(output) <= total Have you tried to figure out a better logic?
(w.r.t this P.R. we can hold off for now. If you figure out why is that line bad, then feel free to open a new PR, if that works well, then we no longer need this one. Otherwise we might have to revisit this one)

Is hasEnoughStorage used for anything else or is it just to check for the StorageFull and add it to the notProducingReasons.

const hasEnoughStorage = isEmpty(output) || used + getStorageRequired(output) <= total * 0.99;

It's a cheap solution and may cause more problems or edge cases? Depends on if the check is used more than just notProducingReasons.

This is indeed a "hacky" solution. Have you tested different edge cases?

Forgot to mention, 0.1.131 contains a "fix" for this. Not sure whether it's a good fix - let's see
fc2b27b#diff-2c0a19611e6cb20343cd8164fabcffb41c4a6bd7e4e58931710dcd2ec9811ebcL347

It's difficult to test the edge cases, with out knowing them and how to get them to happen.

Not sure where stockpileCapacity comes into it, but fixes looks a lot better than mine lol

It's difficult to test the edge cases, with out knowing them and how to get them to happen.

Not sure where stockpileCapacity comes into it, but fixes looks a lot better than mine lol

stockpileCapacity is "Stockpile Input Capacity". So the fix is more aggressive when calculating full - you need space for output AND input * stockpile input capacity to be considered not full.

This seems to be less of an issue now. Will close it for now.