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

Admin interface doesn't provide current company value in SERVER_COMPANY_ECONOMY #4632

Closed
DorpsGek opened this issue Jun 2, 2011 · 14 comments
Labels
flyspray This issue is imported from FlySpray (https://bugs.openttd.org/)

Comments

@DorpsGek
Copy link
Member

DorpsGek commented Jun 2, 2011

xOR opened the ticket and wrote:

Analog to the amount of cargo the company value should be provided with a value for 1. the previous quarter 2. the last quarter and 3. the last month.
Unfortunately 3. is currently missing. What's necessary is probably a
p->Send_uint64(company->cur_economy.company_value);
in the SendCompanyEconomy() function in network_admin.cpp.

I am filing this as a bug (and not feature request) because I think that it was planned to include this information but forgotten - it simply doesn't make sense to leave that out. What's backing me up on that opinion is the fact that the current amount of cargo is also being sent (see network_admin.cpp, SendCompanyEconomy), but not documented in the comments for Receive_SERVER_COMPANY_ECONOMY.
When someone is writing a client based on this documentation he will run into problems, so that should of course also be corrected when the company value is added.

Reported version: 1.1.1
Operating system: All


This issue was imported from FlySpray: https://bugs.openttd.org/task/4632
@DorpsGek
Copy link
Member Author

DorpsGek commented Jun 4, 2011

Rubidium wrote:

The company value field of cur_economy is always empty. Only on a quarterly base the company value is recalculated, so it's simply not available in that field.

Furthermore I don't see the company value not given as a bug as then it would also be a bug that the performance rating isn't given for the current quarter, or the amount of money/loan/incomes in previous quarters.

The issue with the documentation missing information about the delivered cargo is fixed in r22536.


This comment was imported from FlySpray: https://bugs.openttd.org/task/4632#comment9986

@DorpsGek
Copy link
Member Author

DorpsGek commented Jun 4, 2011

xOR wrote:

Well it was just a wild guess that it could be fixed that way. Of course i don't care how exactly it is implemented and whether it's labeled as bug or feature request. I just thought the admin interface shouldn't give a client less information than could be obtained through the console. Anyway, thanks for looking into it and replying. Until this is done it seems I have to stick with console parsing (would confuse too many players if the goal system suddenly lagged behind what they saw in the company information screen by up to 3 game months when I used the quarterly value, I can already hear all the complaints: "it's bugged, it says 30,000 but I already have 50,000 $...") :-P


This comment was imported from FlySpray: https://bugs.openttd.org/task/4632#comment9987

@DorpsGek
Copy link
Member Author

xOR wrote:

In the meantime this bug entry already had its birthday. And nobody brought cake :-(
But seriously, is there nobody who could implement this? The value is shown on the GUI and can be obtained when parsing from the console. So maybe the specific field I have mentioned above is empty, but the value must be somewhere not too far away.
Please show some love for an old and humble feature request <3


This comment was imported from FlySpray: https://bugs.openttd.org/task/4632#comment11293

@DorpsGek
Copy link
Member Author

DorpsGek commented Jul 6, 2012

dihedral wrote:

The value shown in the GUI is calculated by the client. The value you get when connected to the admin interface is calculated by the server. I see no reason why the server needs to be put under high(er) load, in order to recalculate the value for up to 15 companies for up to 15 bots for every poll of the company economy details.

It is also consistent with the UDP Packets OpenTTD sends out, which contain company economy data. The value used here is c->old_economy[0].company_value; same with the performance history being c->old_economy[0].performance_history.

These values were deliberately NOT included in the design of the protocol as it causes to much load to calculate these fields for the server which I cannot justify for the gain.
Waiting until the final value for the running quarter is calculated by the server anyway, rather than causing extra load is what I opted for.

As far as I am concerned this report can be closed.


This comment was imported from FlySpray: https://bugs.openttd.org/task/4632#comment11312

@DorpsGek
Copy link
Member Author

DorpsGek commented Jul 6, 2012

xOR wrote:

Who said it should be provided for every poll? All I asked for is to provide it monthly, so it is consistent with the amount of cargo and other things. And consistent with the company value I can already get from the server console, which IS updated monthly. No client calculations involved there.


This comment was imported from FlySpray: https://bugs.openttd.org/task/4632#comment11313

@DorpsGek
Copy link
Member Author

DorpsGek commented Jul 6, 2012

dihedral wrote:

The 'companies' command run on the console runs "(int64)CalculateCompanyValue(c)" for each and every time you execute the command.


This comment was imported from FlySpray: https://bugs.openttd.org/task/4632#comment11314

@DorpsGek
Copy link
Member Author

DorpsGek commented Jul 6, 2012

xOR wrote:

Don't get me wrong, I think it's a great approach to think about the load impact of every change. I just don't think that doing the calculation 3 times more often will increase the load caused by the OpenTTD server too much. If that report is closed without implementing it I am back to what I did 2 years ago: poll it from the console.
IMHO it would be a lot better if it would be calculated monthly and provided via admin interface instead of making me request it from the console (or via RCON through the admin interface, same thing...) and cause a recalculation each time.


This comment was imported from FlySpray: https://bugs.openttd.org/task/4632#comment11315

@DorpsGek
Copy link
Member Author

DorpsGek commented Jul 6, 2012

dihedral wrote:

as the data can a) be polled or b) be scheduled on a weekly bases, the only clean way in my opinion would be to have the same value as on the console, which is recalculated at the moment of execution.
the additional load would not be 2 times more, but rather up to 180 times more (up to 15 companies, up to 15 admin interface connections, weekly update which is 4x a month, which is 12 x in a quarter) - that equals to 2700 calles in the course of one quarter, compared to 15 calles in the course of one quarter.

again: i am currently not sure i can or even want to be responsible for that possible increase.

What is your aim by the way - perhaps there is another way to accomplish the same?


This comment was imported from FlySpray: https://bugs.openttd.org/task/4632#comment11316

@DorpsGek
Copy link
Member Author

DorpsGek commented Jul 6, 2012

xOR wrote:

The value is used (along with others) by my software to calculate the score of players. Depending on the configuration either this score or directly the company value is used to determine whether a company has reached the goal for the server, hence won the game.

The values are announced regularly (configurable, default every 5 minutes) and along with the announcements also shown on the new Goal dialog:
http://openttd.n-ice.org/images/news/goaldialog.png
Additionally players can write !goal in the chat to see the current rank table at any time.

Players find it irritating that the values shown are always outdated (although I have added a message saying that the values are updated quarterly). It's even more of a problem when the goal is reached. E.g. in close situations it happens that company 1 reaches the goal and has the lead for 2 months. Now in the third month (which happens to be the end of the quarter) company 2 reaches the goal too and even got a higher score than company 1. The result: company 2 has won.
Of course the player in company 1 is now very angry, stating that clearly he had won the goal and that we should "fix our bug". Example:
http://openttd.n-ice.org/images/news/cv-quarterly.png

If it was at least updated monthly it would improve the situation a lot.


This comment was imported from FlySpray: https://bugs.openttd.org/task/4632#comment11317

@DorpsGek
Copy link
Member Author

DorpsGek commented Jul 6, 2012

dihedral wrote:

How about using goal scripts to achieve what you are trying to do? After all, that is what they are actually there for ;-)

If you really want those updates on a monthly basis, I assume you could use a goal script, and push the new values via the available methods to the admin interface (communication between the two parts is done via JSON).

just a personal note:
If I lay down rules and goals for a game, I expect my players to play by them. If the rules is, "roll a 5 with a dice, 3 times in a row", and they only manage to roll a 5 twice and start complaining "that's unfair"... I'm sorry, but the rule is quite clear, is it not?
If my rule says "have 40.000.000 $ company value, calculated quarterly, as the graphs in the game are updated also" I think that is a very clear statement, do you not? ;-)

As for the monthly update, it's either realtime data or none at all - I will check it with other parts of the code making use of that function and rethink my opinion on that matter.


This comment was imported from FlySpray: https://bugs.openttd.org/task/4632#comment11320

@DorpsGek
Copy link
Member Author

DorpsGek commented Jul 6, 2012

xOR wrote:

I am already using a goal script to push values over the admin interface, so if the value was there it would be a matter of minutes to implement it. But on the Company object I only find a GetQuarterlyCompanyValue() function. Give me a GetCurrentCompanyValue() function and I am happy :-)

In theory I might be able to calculate the value myself on the script (iterate through the vehicles, find out their value, add the money, subtract the loan...). But apart from the fact that it already SOUNDS plain stupid to re-implement OpenTTD's value calculation algorithm in a script run by it... it would probably really have a bad performance impact when doing that from an interpreted script. If that's the only possible solution just querying and parsing it from the console will be a better option.

About your personal note: "quarterly" might be normal in business and economy world but in normal life you don't meet it that often. It just doesn't feel natural if something happens quarterly. I am not receiving my salary quarterly. I don't pay my insurances quarterly. Hell, I don't even notice switches from one quarter to another if nothing at my office depends on it. But I always notice when we go into a new month.
Sure, the announcement text says it. You and me understand this. But in reality people don't read texts and just "assume". So for example they assume that when they reach the values set as goal then this would mean that...well...they reached the goal :-)


This comment was imported from FlySpray: https://bugs.openttd.org/task/4632#comment11322

@DorpsGek
Copy link
Member Author

DorpsGek commented Jul 7, 2012

dihedral wrote:

The more time I spend looking at cur_economy, and company_value and how it is used, the more I come to the conclusion that I should not change it.
It is consitent throughout the game and I will not change that consistency - the only 2 places realtime data is presented is in the console command 'companies' and in the company detail window on the client side. I only consider it 'fair' that an admin has the chance to see the same value on the console, as the clients in the company details window.

however, you could patch your server, e.g. with the attached patch, which adds a uint64 at the end of the COMPANY_ECONOMY Packet with the realtime data (making use of CalculateCompanyValue()). Though i highly doubt it will entirely remove any complainst from your players in that respect.

Again: as far as I am concerned this report can be closed.

Attachments


This comment was imported from FlySpray: https://bugs.openttd.org/task/4632#comment11323

@DorpsGek
Copy link
Member Author

DorpsGek commented Jul 7, 2012

xOR wrote:

The whole idea of doing everything from an external application was to avoid patching. Appreciate the effort but any solution doesn't help me as long as it doesn't go into the base application.
Also the way it was done on the patch means even people who don't want the live data will cause recalculations for all company values, possibly without even being aware of it.

I'd rather propose to just offer it and let the interface user actively doing a decision on it. For example providing a CalculateCompanyValue() function to GameScripts would be a more clean solution and by the name it would be clear that it's causing a live calculation on the server. This way people using the interface can do the decision about balancing between load and currentness of data by themselves. Furthermore you additionally provide that feature to GameScripts too, not only to admin interface clients.

From effort perspective it shouldn't be hard to do, only add a new GameScript function and make it call the internal (int64)CalculateCompanyValue(c) function you mentioned and return this data. And from load perspective: there is no load added by default. Only if a GS writer decides to do the calculations, knowing about a possible load penalty.
So do you think this is something that could be done? Could it have a chance if I opened an extra feature request entry here for this?


This comment was imported from FlySpray: https://bugs.openttd.org/task/4632#comment11334

@DorpsGek
Copy link
Member Author

andythenorth closed the ticket.

Reason for closing: Won't implement

Flyspray clean up: more than 5 years old, and not obvious what should be done with this next, so closing. If this offends, discuss with andythenorth in irc. Thanks.


This comment was imported from FlySpray: https://bugs.openttd.org/task/4632

@DorpsGek DorpsGek added Admin flyspray This issue is imported from FlySpray (https://bugs.openttd.org/) labels Apr 7, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
flyspray This issue is imported from FlySpray (https://bugs.openttd.org/)
Projects
None yet
Development

No branches or pull requests

1 participant