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

Upgrading from a version prior to 0.7.3 changes specific use cases using vehicle random_bits #350

Open
heresyh opened this issue Dec 28, 2024 · 3 comments

Comments

@heresyh
Copy link

heresyh commented Dec 28, 2024

Hello,

I recently upgraded from 0.7.1 to 0.7.5, and when I re-compiled a GRF (Mop's Expanded Road Vehicles) I noticed that some vehicles in-game had broken sprites.
I narrowed it down to this change, #288, after reading through the changelogs and testing different versions/commits.

This is due to how the original code is in deciding between different spritesets, f.ex:

switch (FEAT_ROADVEHS, SELF, switch_w_l_f_flat_e, random_bits ) {
    0..63:         spriteset_w_l_f_flat_e;
    64..127:       spriteset_w_l_f_flat_f;
    128..191:       spriteset_w_l_f_flat_g;
    192..255:       spriteset_w_l_f_flat_h;
}

As there is no default value or specific range defined there is no valid choice when the random value is between 256-65535.

I don't know if any other GRFs have the same issue, or if this is a very specific issue to how this GRF was written.
However, I do think it would be nice to have this show up as a warning in the nmlc output, if possible, so that its easier to catch if this is a problem in their code.

It also changes the probability between choices in cases like this, where it will now heavily favour the default value when compiled with a newer version of NML:

switch (FEAT_ROADVEHS, SELF, switch_o_box_b, random_bits ) {
        0..69: spritegroup_o_box_a;
        70..139: spritegroup_o_box_b;
        140..209: spritegroup_o_box_c;
        210..255: spritegroup_o_box_d;
        default: spritegroup_o_box_b;       
}

However, that is less of an issue, as it at least does have a default value to fall back to, even though it might lead to an unknown/unwanted change in behaviour for other GRFs, if they have implemented logic like this, and they are unaware of the random bits change.

@PeterN
Copy link
Member

PeterN commented Dec 28, 2024

You should mask random_bits to the number of bits you need. random_bits & 0xFF

@WenSimEHRP
Copy link

A neater solution is to use a random switch instead of a switch

@heresyh
Copy link
Author

heresyh commented Dec 28, 2024

You should mask random_bits to the number of bits you need. random_bits & 0xFF

Already changed things in my own fork, issue is more about lack of information about what might be wrong if someone updates to a newer NML version and things silently breaks or changes without any warnings or pointers to what is the cause.

I don't know if any other GRFs actually have coded in randomisation the same way it was done here by the original author, but I figured I'd make an issue about it at least, in case someone else stumbles upon the same.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants