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

GSStation::GetCargoRating always returns 69 for cargo IDs that don't have a rating #5514

Closed
DorpsGek opened this issue Mar 27, 2013 · 9 comments
Labels
component: AI/Game script (squirrel) This issue is related to Squirrel (Scripting language) flyspray This issue is imported from FlySpray (https://bugs.openttd.org/)

Comments

@DorpsGek
Copy link
Member

xOR opened the ticket and wrote:

If GSStation::GetCargoRating is used on a valid station ID and with a valid cargo ID that the station doesn't have any rating for (because that cargo is not transported there) the function will always return 69. A script calling this function cannot know whether the returned value of 69 means "no rating" or "rating of 69%".

This is especially a problem because for a script there seems to be no reliable way to find out whether a given cargo is transported at a station. What I am doing right now is to do additional checks if the rating is 69:
- is this cargo type currently waiting at the station? if yes, then assume the rating means 69%, otherwise continue
- iterate through vehicles in GSVehicleList_Station and check whether GSVehicle::GetCapacity of that vehicle for the cargo type is > 0? if yes, then assume the rating means 69%, otherwise continue
- if none of the above apply assume the rating means "no rating"

My expectation would be that the function returns -1 or something represented by an INVALID_RATING constant if there is no rating. Alternatively an additional GSStation::HasCargoRating function would do the job.

Reported version: 1.2.3
Operating system: All


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

xOR wrote:

As I haven't seen any actions regarding this in the changelogs and didn't find anything on this bug tracker I assume this problem also exists in 1.3.0-RC3 but as I only tested it on 1.2.3 I set this as "Reported Version".


This comment was imported from FlySpray: https://bugs.openttd.org/task/5514#comment12123

@DorpsGek
Copy link
Member Author

krinn wrote:

It has been like that for more than just that version, in noai too.
Maybe you misuse the function, as you are trying to check the rating of a cargo to see if a given cargo is transported there, while a cargo rating is only apply to cargo distribution : so to get cargo not drop cargo.
You can easy see it : transport coal from X to Y : X get a rating, Y get no rating at all.
And there's a default "no rating yet" set, because even not yet rate the station cargo get influence by ingame options : like buying transports rights, success and failure of bribing, publicity campain, and maybe some others game mechanics (plane crash...)


This comment was imported from FlySpray: https://bugs.openttd.org/task/5514#comment12125

@DorpsGek
Copy link
Member Author

xOR wrote:

I am using it from a script that calculates XP for players based on certain values. It is similar to the company performance value but takes more things into account. And one of these things is the average cargo rating a company has, for which to calculate I need to get every cargo rating the company has at every station. So I am NOT using it to see if a given cargo is transported there, I am using the "GetCargoRating" function to...well...get the cargo rating. But as I am doing it at a given instant I start by iterating through all stations the company has and then through all cargo IDs known on the game.
A way to solve this would be to use the cargo monitor and check and store the ratings every time there is some sort of movement, but that would mean a lot of overhead and constant unnecessary code execution.

If the idea is that every function should only be used for the one use-case a developer had in mind when implementing it, then yes, I am most probably "misusing" or even "abusing" every single GS function and admin interface feature the game provides. And I don't even feel ashamed of myself, I boldly call it creativity.

I understand your explanation about the internal calculations that need a base value, but then please consider my suggestion of implementing an extra HasCargoRating function.


This comment was imported from FlySpray: https://bugs.openttd.org/task/5514#comment12126

@DorpsGek
Copy link
Member Author

krinn wrote:

I understand your need for an HasCargoRating function now.
For your case, you could, reduce calculcations by checking if the station is providing any goods, removing any checks for a receiving only station where rating has no meaning at all. I dunno, check it against GSIndustryType.IsRawIndustry per example.

"If the idea is that every function should only be used for the one use-case a developer had in mind when implementing it, then yes, I am most probably "misusing" or even "abusing" every single GS function and admin interface feature the game provides. And I don't even feel ashamed of myself, I boldly call it creativity."
Well, if you feel a function need to do something it is not doing, it's a feature request.
But if you feel the function isn't doing what it is suppose to do, it's a bug.
Don't report it as a bug because the function block your creativity :)

Something to consider also is that such a feature, if not yet implemented in openttd, would add overheat to the game itself, to remove it on your GS. So if openttd handle that itself already, there's chance you'll get a HasCargoRating added.
But if openttd doesn't track that because it's not of any use, it would be costy to add it just for some GS to make some stats.
Something an openttd dev will answer you.

You can reduce a bit your current test already :
- iterate through vehicles in GSVehicleList_Station (filtered by GSVehicle.GetLocation == GSStation.GetLocation) and check whether GSVehicle::GetCapacity of that vehicle for the cargo type is > 0? if yes, then assume the rating means 69%, otherwise continue
Avoid to loop all vehicles that are using the station, as if the station is at 69 rating and doesn't have any cargo, it must be because someone is loading the cargo.

I think you should solve it by tracking stations yourself, using industry opening/closing event to detect updates. This way, you will loop only stations that are "producing" stations, and only for the knows cargos the station produce and not all the cargos in game. Reducing the overheat by a huge factor.


This comment was imported from FlySpray: https://bugs.openttd.org/task/5514#comment12127

@DorpsGek
Copy link
Member Author

xOR wrote:

If you ask a function for a rating and there is no way to distinguish one case (no rating) from another (69%) I DO think it definitely is a bug, regardless of what I use the function for and what internal technical reasons its behavior might have. Also there is no way someone using that function the first time could expect that, it cost me quite some time to figure out what is happening as this "weird" behavior is not documented. And I really hope this bug is not "solved" simply by documenting it :-(

Anyway, I appreciate your explanations and your input for ideas about how to improve my code around this. It's not that I am completely lost right now, especially when for scoring it doesn't need to be perfect: it's only one of many parameters and the calculation is the same for all companies, so it's still fair even when it contains some degree of inaccuracy. It's just that I still think this function is behaving wrong by its documentation and by what the name implies it would do.


This comment was imported from FlySpray: https://bugs.openttd.org/task/5514#comment12128

@DorpsGek
Copy link
Member Author

krinn wrote:

That's where you mistake : there IS NO two cases with a no rating and another 69%. The two cases are you have a station or not. If you have a station you have a rating !
You have a rating as soon as you have station, that rating will vary if you use the station, but also even if you don't use it, as i have told you, by bribing, publicity...

So 69% is just a per default 0 influence value, it's not your real station rating. If you fail at bribing a town, all stations will get their rating modified because of that, even the ones you don't use.

If you want see that, just do that :
- Build a bus station in a town (name it X)
- Bribe town until you get caught
- Build another bus station, but only after you get caught ! (name it Y)
- Build a bus and send it to X and Y
Y rating default is 69%
X rating default is 4% (more or less, depend on how much time your bus tooked to reach it), anyway far from your supposed 69%

Now if you ask the function, without any buses running it would tell you 69 for Y and 4 for X : and these are correct values == no bug there, sorry.

So if anyone change the function like you wish, all users that check their cargo rating within their station before using it will complain the function is bug now.


This comment was imported from FlySpray: https://bugs.openttd.org/task/5514#comment12129

@DorpsGek
Copy link
Member Author

xOR wrote:

My problem is about the different cargo types a station can have - for a bus station it's quite obvious, so not a good example.

Here's another one:
Imagine I got only one station where I transported wood only once, but then never again until the rating for wood dropped to 30%. From OpenTTD client I check the rating and see only this: "Wood: 30%". Clearly the average of a single rating of 30% is 30%.

But if I iterate through all stations and cargo types from the script and calculate the average the script will see:
Wood: 30%, Coal: 69%, Steel: 69%, Diamonds: 69%, something else: 69%, something else: 69%, something else: 69%, something else: 69%, something else: 69%, something else: 69%, something else: 69%.

So the script calculates a total average cargo rating of 65%. But I want to get the 30%.

"So if anyone change the function like you wish, all users that check their cargo rating within their station before using it will complain the function is bug now." <-- That might be true for programmers, or is that what you mean by "users"? If players would check the ratings for their wood station and see that they got a rating of 69% for coal, THIS is where they would complain about a bug ;-) But what they see instead is that they got no rating for coal on that station - something that a GameScript can not see.
As I said earlier, I agree to not change the function if internal mechanics rely on it. Though I wonder about that, because in all other cases some WHATEVER_INVALID constants are used if something wasn't initialized and doesn't exist. And if for some calculations you need a base value you can still easily treat your WHATEVER_INVALID constant as 69 in those specific situations.
Anyway, if the function cannot be changed the "69 magic" should at least be documented and there should be some alternative to find out whether a station has a rating for a given cargo, like any user of the GUI client can.


This comment was imported from FlySpray: https://bugs.openttd.org/task/5514#comment12130

@DorpsGek
Copy link
Member Author

DorpsGek commented Apr 6, 2013

Rubidium closed the ticket.

Reason for closing: Fixed

In r25150


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

@DorpsGek DorpsGek closed this as completed Apr 6, 2013
@DorpsGek
Copy link
Member Author

DorpsGek commented Apr 6, 2013

xOR wrote:

Read the r25150 checkin comment, great stuff, thank you Rubidium!


This comment was imported from FlySpray: https://bugs.openttd.org/task/5514#comment12148

@DorpsGek DorpsGek added flyspray This issue is imported from FlySpray (https://bugs.openttd.org/) Goal/Game script labels Apr 7, 2018
@frosch123 frosch123 added the component: AI/Game script (squirrel) This issue is related to Squirrel (Scripting language) label Apr 14, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
component: AI/Game script (squirrel) This issue is related to Squirrel (Scripting language) flyspray This issue is imported from FlySpray (https://bugs.openttd.org/)
Projects
None yet
Development

No branches or pull requests

2 participants