Bug #7496

Script produces non-feasible goals

Added by Alberth over 4 years ago. Updated over 4 years ago.

Status:NewStart date:2015-02-15
Priority:NormalDue date:
Assignee:-% Done:

0%

Category:-
Target version:-

Description

Oil refinery that should get iron ore

http://www.tt-forums.net/viewtopic.php?p=1142726#p1142726

History

#1 Updated by krinn over 4 years ago

It is because you use your cargo_id (the real cargo id) as a reference to get cargo class from cargoes table.

But the cargoes table is not made of cargo_id, but made with an increase value for each founded cargoes (a more good name would be cargo_index imo).

It mean cargoes could be made of :
[0 (the cargo_index)] = cargo class -> with cargo_id = 0
[1] = cargo class -> with cargo_id = 2
[2] = cargo_calss -> with cargo_id = 4
...
example:
line29 company.nut local goal_text = GSText(GSText.STR_COMPANY_GOAL, cargoes[this.cargo_id].cid, this.wanted_amount, destination_string);
cid is indeed the cargo_id of the cargo, but this.cargo_id should be rename to cargo_index to remove ambiguity.
Didn't look in the code if someone has pass a real cargo_id instead of cargo_index, but this is something that sure will happen.

And if this happen (someone pass cargo_id = 2) cargoes[this.cargo_id].cid is not equal to the cid = 2 but equal to cid = 4 (the cargo_id#4 from the store cargo class index#2)
And cid = 4 could be iron ore, while cargo_id was mean to aim at cargo #2 (petrol).

If you remove the ambiguity, i think you will find the bug where cargo_id of a real cargo_id was given instead of its cargo_index value.

#2 Updated by krinn over 4 years ago

Oops, i see your current revision already removed the ambigious naming to "cargo" instead of cargo_id

#3 Updated by Alberth over 4 years ago

Yes I did (hopefully all correct). Anyway, if there was such an error, I would expect the other refineries in the same screenshot also to be wrong, which is not the case.
For this reason, the industry-changing thing seems more likely to me at the moment.

Thanks for looking into the problem.

#4 Updated by krinn over 4 years ago

To makes something a few worth my previous post :)

Even the industry changing looks the best so far, all my tests with AI or GS i've done never shown a new industry taking old industry id that fast.
You have event to catch industry closing, so in order to switch industry old one should close and new one open with the same id while you didn't catch yet the event.

I don't know the openttd internal to assign new id on new industry, so i'm not sure if it have some kind of "delay" before reassign old id to a new industry, but as openttd must keep old id for some time (at least to answer to anyone asking the id of the close industry or display message...), there's a delay before assigning old id to a newer one so.

If your script was busy that much that a huge delay makes the old id removed and new industry build with the old id, your event handling should had also been so much delay that when it handle the "industryclose event", it should get the id of the new industry, making it thinking the new one is closing (and so removing the goal too).

I think the race condition should be more this than industry switching: assigning a loop with industries list and in that list a closing industry is there (closing message was send and process already, but the industry is still valid for few few time still), and script sadly took this one.

Without savegame, both conditions will be almost impossible to reproduce, i think you could just wait another report to see if it's something else ; when i have few times i'll look again at code (but the latest release this time).

Also available in: Atom PDF