Page 1 of 1

Important! The GRF search!

Posted: 24 Jan 2008 04:29
by George
2 All the industry GRF developers!!!

We (me and belugas) found a serious discrepancy between the understanding of the documentation. So, we are looking for the GRF author, who already coded the GRF with following assumption for callback 35/29:

When industry var 93 is EQUAL to 4 (and 0-3 too, but it is not a question for discussion), that means the industry closure! (As belugas sees it)

If you see that industry var 93 is EQUAL to 0-3 means the industry closure, while value 4 does not mean it (As I see it), please, write it here too

Re: Important! The GRF search!

Posted: 24 Jan 2008 05:22
by DaleStan
My view is that 04 does not indicate closure, and the documentation is misleading. A more correct statement would be "If this value [01] is returned when the production is a quarter of the default, the industry is closed instead, as if you returned 03."

What happens in TTDPatch? When it comes to GRF handling, the burden of proof is on the side that claims TTDPatch is wrong.

For more fun-and-games, during a recession, all production rates are divided by two, and the lower limit is reduced to 02.

Re: Important! The GRF search!

Posted: 24 Jan 2008 06:52
by George
DaleStan wrote:My view is that 04 does not indicate closure, and the documentation is misleading. A more correct statement would be "If this value [01] is returned when the production is a quarter of the default, the industry is closed instead, as if you returned 03."
This exactly my point
DaleStan wrote:For more fun-and-games, during a recession, all production rates are divided by two, and the lower limit is reduced to 02.
Sorry, but my tests shows that this is not correct. It does not change the production level (var 93), but halves the output, and halves it AFTER it was generated by the industry AFTER production callback. This is very impotant for ECS vectors, because the industries uses this behaviour and do not need to provide additional handling for recession. If var 93 would change (/2) during recession, I would need to rewrite every industry code :roll:

Re: Important! The GRF search!

Posted: 24 Jan 2008 07:42
by DaleStan
Well then, I stand corrected. At least I remembered the "halved" bit correctly. I just forgot when the halving happens.

Re: Important! The GRF search!

Posted: 24 Jan 2008 16:18
by belugas
I would like to bring in my side of the situation.

The var 93 (OTTD = industry->prod_level, TTDP = industry.prodmultiplier) is used for controlling the production of the industry.
It is been re-evaluated once every month. During the evaluation, callbacks are been fired and results will affect var 93.

In OTTD and in TTDP, from what we have seen code wise, if ever the var 93 reaches 4, at the end of the evaluation procedure, the system will lower the var 93 to 0 and leaves. This means that during the evaluation of the next month, the system will effectively close that industry. So it basically gives a full month for the industry to close.

Code: Select all

TTDPatch:
.decrease:
.decloop:
	cmp byte [esi+industry.prodmultiplier],4
	je .closedown
	shr byte [esi+industry.prodmultiplier],1
	add byte [esi+industry.prodrates],1
	sbb byte [esi+industry.prodrates],0
	shr byte [esi+industry.prodrates],1
	add byte [esi+industry.prodrates+1],1
	sbb byte [esi+industry.prodrates+1],0
	shr byte [esi+industry.prodrates+1],1
	loop .decrease

OpenTTD:
	/* Decrease if needed */
	while (div-- != 0 && !closeit) {
		if (i->prod_level == 4) {
			closeit = true;
		} else {
			i->prod_level >>= 1;
			i->production_rate[0] = (i->production_rate[0] + 1) >> 1;
			i->production_rate[1] = (i->production_rate[1] + 1) >> 1;
			if (str == STR_NULL) str = indspec->production_down_text;
		}
	}
I post in here the code OTTD uses for this purpose:

Code: Select all

void IndustryMonthlyLoop()
{
	Industry *i;
	PlayerID old_player = _current_player;
	_current_player = OWNER_NONE;

	FOR_ALL_INDUSTRIES(i) {
		UpdateIndustryStatistics(i);     <----- does not affect prod_level/ var 93
		if (i->prod_level == 0) {         <----- has it been declared for closure?
			delete i;                         <----- yes, close it now
		} else {                                 <-----  no, let';s evaluate it
			ChangeIndustryProduction(i, true);    <------  evaluation been performed in here. [0]
		}
	}
	...
[0] So, if prod_level == 4, it will be shifted to 0 right away, in orderto be able to be closed in next month loop.
The ChangeIndustryproduction function holds this code, near the end of it, that actually does the above mentionned behaviour

Code: Select all

	/* Close if needed and allowed */
	if (closeit && !CheckIndustryCloseDownProtection(i->type)) {
		i->prod_level = 0;
		str = indspec->closure_text;
	}
Unless otherwise proven, we are convinced TTDP does exactly the same process.
So, all in all, it means that if ever var 93 reaches 4, the industry will be closed next month.

Re: Important! The GRF search!

Posted: 24 Jan 2008 16:44
by DaleStan
belugas wrote:In OTTD and in TTDP, from what we have seen code wise, if ever the var 93 reaches 4, at the end of the evaluation procedure, the system will lower the var 93 to 0 and leaves.
That's not what I see.

Code: Select all

TTDPatch:
.decrease:
.decloop:
	cmp byte [esi+industry.prodmultiplier],4
	je .closedown
	shr byte [esi+industry.prodmultiplier],1
Compare with 4, and then reduce the multiplier. So you can reduce it to 4, industry.prodmultiplier will not be zeroed, and the industry will stay open. At least until next time this code is seen.
As for "next time", the label name .decrease and the code that jumps to .decrease both make me very suspicious that .decrease is not executed unless the callback specified that the industry will get a production decrease.
And then there's the code in the loop; there's no code there to fail to change the production multiplier, so if it gets executed when a decrease is not intended, there's no way to undo the decrease. .increase, on the other hand, does not check to see if industry.prodmultiplier is 4, nor does .nothing or .error.

Re: Important! The GRF search!

Posted: 24 Jan 2008 18:20
by belugas
Thanks to frosch, glx and Dalestan, something a bit odd ( :oops: ) has been spotted in the 0D/0E result handling of callback 29/35.
On revision r11976, i've made a little change. I hope it will make a big difference.
The evaluation of the closure was not done properly and made it closing too quick.
I would appreciate feedbacks on it.
Thanks

Re: Important! The GRF search!

Posted: 31 Jan 2008 03:57
by belugas
So? Did it made the difference?
Was it worth it?

Re: Important! The GRF search!

Posted: 31 Jan 2008 04:23
by George
belugas wrote:So? Did it made the difference?
Was it worth it?
Yes, the fix works fine at least in my tests. There were no bug reports about unexpected closure of the mines any more, so I hope it works fine for other users too. Thank you, Belugas :D

Re: Important! The GRF search!

Posted: 01 Feb 2008 15:01
by Roujin
It definately was worth it. After that fix, the issues i had with mines in george's ECS vectors dissolved :)

Thanks again ;)