-
Notifications
You must be signed in to change notification settings - Fork 38
Add: Support for NewGRF badges. #359
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
base: master
Are you sure you want to change the base?
Conversation
8132947 to
df390e8
Compare
40aab42 to
955f470
Compare
955f470 to
13fa70a
Compare
|
Checking badges on a nearby tile for now would require something like this better if we can simplify it to whereas |
|
action 0 flags... |
|
More than happy to push these patches to my fork for cherrypick, but eh, they're tiny. |
| } | ||
| # Railtypes have no 60+x variables | ||
|
|
||
| varact2vars60x_railtype = { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Most of the variables are missing from NFO docs. This one is also missing from OTTD implementation at least for rail/road/tram types.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This appears to be implemented now (Iron Horse uses has_badge on railtypes).
|
Been using this for Iron Horse since the PR opened, works for me. |
1628ea4 to
662945b
Compare
|
|
||
| def get_badgelist_action(badge_list): | ||
| index = 0 | ||
| actions = [] |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Item blocks can be inside if/else blocks, but badgetable can't, therefore NewGRF autor is not able to provide backward compability in NewGRF that uses badges.
Adding act9 which skips badgetable would prevent NewGRFs with badgetables from crashing on older OpenTTD versions.
+ if len(badge_list) <= 0:
+ return []
+
index = 0
- actions = []
+ actions = [action7.SkipAction(9, 0xA1, 4, (0x04, r"\7<"), 0x1F000000, len(badge_list))]PS. I have tested this code snippet and it works as intended with 13.3 and 15.0-beta3.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Well if you are using badges, backward compatibility is not expected. Just target OpenTTD 15+.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I would rather support backward compatibility than badges, but I'm able to compile nml myself, so it isn't a big deal for me :). Just because 15.0 also adds bridges over stations I already have the station item blocks inside if blocks. It is a shame a litle bit that the badgetable can't be inside conditional block.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
+1, we don't need to skip badges, backward compatibility isn't a good goal for grf.
|
No warning/error is thrown when creating a FEAT_BADGES item, whereby the label is not in the badgetable. Unless I'm msissing something here, I can't think why I would define a badge item that isn't in the badge table (and therefore isn't used on a vehicle? I'm not sure if that should be considered an error, but it certainly feels like something that should trigger a warning since it's most likely a typo |
|
A similar "A warning would be nice" situation If I have a badge eg "power/steam", but no "power" badge defined, the "power/steam" badge does show up in the newGRF debug menu but doesn't show up in the purchase menu as either a sprite or text. If the user's intention was to add text that's kinda obvious ("Power: steam" can't displayed if "Power" doesn't exist to have text), but it's a little counter-intuitive if the user is mostly thinking about adding a sprite but didn't want text I assume this is a limitation of the implementation in OpenTTD rather than NML, but it would be good if NML threw a warning if a nested badge was defined without a parent badge already being defined |
Much like defining railtypes and roadtypes, there's no requirement to use badges when defining them. Maybe you wanted to make a collection of badges to be used in other NewGRFs. A badge table is only required to use badges, not to define them. It's also not an error to not define a class badge. |
Lots of other NML warnings aren't really errors, either ... that's the point of a warning, surely? "You might be doing this intentionally but there's a good chance you aren't, maybe take a look?" |
662945b to
aa5c419
Compare
|
Fixed the naming of flag |
Instead of a boolean to toggle using an extended-byte, len_size allows byte, word, extended-byte and dword types.
aa5c419 to
a458814
Compare
a458814 to
8f6c3b6
Compare
| BlockAllocation(0, 62, "Roadtype"), | ||
| BlockAllocation(0, 62, "Tramtype"), | ||
| BlockAllocation(0, 0xFFFE, "RoadStop"), # UINT16_MAX - 1 | ||
| BlockAllocation(0, 64000, "Badge"), |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why the limit here is only 64000 and not UINT16_MAX like the other limits?
Especially when the badge_id in the OpenTTD source code is uint16_t.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Because if you ever need an extra 'special' ID for some feature, having a range of IDs that are unused is way easier than lowering the limit as you need to write extra logic to remove the badges with that special ID.
Even then, if 64000 seems way too much to me. The idea it to be able to filter with them, and if you need more than a few hundred I'd argue that the categories are way too fine grained.

Add support for NewGRF badge feature in OpenTTD/OpenTTD#13073
Use
badge_table { }to create the translation table.Use
badgesproperty to assign a list of badges to an entity.See 042_badges.nml for slightly more info.