Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Station correcting overload causes missing cargo warning #252

Open
AVeenstra opened this issue Feb 13, 2021 · 5 comments
Open

Station correcting overload causes missing cargo warning #252

AVeenstra opened this issue Feb 13, 2021 · 5 comments
Assignees
Labels

Comments

@AVeenstra
Copy link

Intro

Love your mod!
I tried to make this bug report as detailed as possible, please forgive my mistakes and keep up the great work!

Bug description

If a train is overloaded at a station and the overloaded items are then removed again, then the ltn-message.provider_missing_cargo might appear even if the actual contents are still more than the original request.

Lines causing the warning:

https://github.com/Yousei9/Logistic-Train-Network/blob/1d237981a0da80b6d6911d5a589f77ee4a7bd2cf/script/train-events.lua#L183-L186
https://github.com/Yousei9/Logistic-Train-Network/blob/1d237981a0da80b6d6911d5a589f77ee4a7bd2cf/script/train-events.lua#L213-L216
I don't think this causes the problem.

Lines updating the delivery:

https://github.com/Yousei9/Logistic-Train-Network/blob/1d237981a0da80b6d6911d5a589f77ee4a7bd2cf/script/stop-update.lua#L289-L293
Introduced in:
https://github.com/Yousei9/Logistic-Train-Network/blob/da3676c944be490aa8164a4783248b379ee44cd0/control.lua#L1253-L1257

The commit message was:

- overloading trains updates delivery size once per update interval (before update was only done once train leaves provider)

The previous behavior describes what I expect the mod to do, the updated behavior describes the cause of the bug (in my opinion). I am not sure where to find the reason of this change. Please forgive me if I missed something obvious.

The delivery is increased to match the overload whenever it occurs, which means that any overload correction (reducing the contents of a train) by the player will generate warnings even if the contents is more than the original request.

Expected behaviour

The station provides the requested amount, so anything equal to or more than than that should be acceptable. This is why I do not expect the missing cargo warning.

Reducing the train overload is important if the requested item is costly to produce. You would not want to overload nuclear reactors.
Also, a station with both loading and unloading is easier to create if it does not have to check if an item is unnecessary or just overloaded.

To Reproduce

Savegame with example station: ltn-test.zip
Might not provide instant results, so please wait for about 3 deliveries.

Alternatively, create a station that loads with (stack) inserters and unloads the overloaded materials.

LTN version

1.15.2

Log file

factorio-current.log
Extra logging of all deliveries included. Also logs the expected vs. actual amount if there is missing cargo.

Lines added to dispatcher.lua:24:

  for train_id, delivery in pairs(global.Dispatcher.Deliveries) do
    log("Train: " .. train_id .. " Has schedule: " .. serpent.line(delivery.shipment))
  end

Line added to train-events.lua:184:

              log("Train: " .. trainID .. " Item: " .. name .. " Should have: " .. planned_count .. " Has: " .. count)

Timestamps of an erroneous run:

  • Delivery created: 774.281
  • Overload correction: 782.859
  • Warning creation: 785.626
@0ptera
Copy link
Owner

0ptera commented Feb 13, 2021

That's some interesting interaction between alerts and some 3 year old code.

If I didn't update delivery with train inventory during stop update LTN will not see overloaded shipments in time resulting in it sending a train to pick up items already loaded in the first train.

Update:
Fixing this error would require keeping track of original delivery sizes and comparing train inventory to those for underload alerts.
I don't feel like fixing this alert warrants duplicating the whole delivery data structure.

@AVeenstra
Copy link
Author

Thank you for the explanation! That makes sense.

Sorry for the slow reply.

Might I be so rude as to suggest possible fixes (not in any particular order):

  • Remove the missing warning altogether.
    Easy fix, but someone else will probably complain.
  • Add a field to the shipment. Upon overfill set it. Disable the warning if the field is set.
    Adding a field just to solve this edge case might be a bit overkill and an actual underload might still occur.
  • Add an option to disable this specific warning.
    There are quite a few settings already, so it is probably best to avoid adding more.
  • Only warn if the item is missing altogether, so if the count is zero.
    Your comment suggests LTN will gracefully handle overloading. The logs suggest underloading is not really that much of a problem as the delivery is updated on departure and the next delivery will fix the discrepancy. Only repeatedly underloading would be a problem, but that is too hard to detect anyway. The easiest thing to detect, and the only thing LTN can not fix with more trains is if the station does not load the items. That is why this option is my favorite.
  • Don't fix.
    Personally I ignore all warnings by now due to the false positives, so this is my least favorite option. However, I can remove the warning in my own copy of LTN, so in the end everyone will be happy.

Some of these I might be able to implement myself (certainly the last one), so let me know if you prefer a pull request.

@0ptera
Copy link
Owner

0ptera commented Feb 15, 2021

Best solution would be if you built a station loading only requested amount without removing items.

@Kurocon
Copy link

Kurocon commented Mar 4, 2021

I'm running into this in my game as well, the missing cargo warnings are pretty annoying and are also incorrect. They don't reflect the original requested values, which are satisfied properly. So this seems like an actual bug worth fixing in the mod.

I've implemented the suggestion you've made above in a local clone (the solution of keeping the original request info alongside the inventory), and it seems to be working fine. Let me know if you'd like a PR for the changes.

@DaleStan
Copy link

I'm also running into this. A loading station that overloads and then corrects may be wrong, but the existing message is also wrong.
If loading stations must not overload-and-correct, a new message should be introduced that says something like "Underload tracking disabled because a requested item was removed at the loading station".

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

4 participants