Animation fixes

[Csaboka]

Hi folks!

Attached are my first[1] attempts at changing OTTD code. I hope they are in the right format, but feel free to apply a clue-by-four if they're not. I may be an experienced TTDPatch developer, but I'm still a newbie at OTTD coding

The first patch fixes a bug Rubidium has accidentally introduced with the new flexible animation array, and could cause memory overwrite when loading an old save game.

The second tries to revert the logic of the animation handler to the original TTD logic. The memmove may seem to be an overkill, and the original logic more convoluted, but this is to ensure that all tiles are always animated, and the animation order stays consistent. I hope the comments explain what the change does exactly.

[1] OK, actually second because I've started the smooth production changes patch, too, but that was long ago.

[Edit] fixed the first patch, there was a return statement missing

[Draakon]

it is good to see that even Patch devs are helping OTTD.

[belugas]

Thanks Csaboka.
Rest assure, you are doing quite fine Code style is flawless.
I think i'll commit it as soon as i can.

[Csaboka]

Rest assure, you are doing quite fine Code style is flawless.

Hehe, Visual Studio doesn't think so: "LoadOldAnimTileList: Not all code paths return a value"
I've forgot to add a "return true" statement to the end, so it returned an undefined value when successful. I've edited the first post to fix the patch.

While testing, I've also found a minor off-by-one error in the industry tile animation handler. You can check it by looking at Pikka's oil wells. The "nodding donkey" pumps have a tiny pause in every cycle because their last stage lasts twice as long. That's because the code initializes num_frames as the real number of frames, but then assumes it contains one less than the number of frames. I've modified the initialization rather than the usage because that's how the same code is written for houses, so it's at least consistent, although a bit confusing.

[Born Acorn]

You can check it by looking at Pikka's oil wells. The "nodding donkey" pumps have a tiny pause in every cycle because their last stage lasts twice as long.

I actually noticed this myself. I just dismissed it as some sort of limit.

[belugas]

Rest assure, you are doing quite fine Code style is flawless.

Hehe, Visual Studio doesn't think so: "LoadOldAnimTileList: Not all code paths return a value"

Hey... I've only talked about code style
I cannot do anything compilation and testing wise from work, thus waiting to get home (and family put to sleep) in order to work on it.
Using Visual Studio myself, it would have shown too

[Csaboka]

I see you've committed the off-by-one animation fix, thank you.

Can I ask why you didn't commit the remaining two patches? I can accept if you devs decide they shouldn't go to the trunk, I'd just like to know the reason.

[TrueBrain]

I see you've committed the off-by-one animation fix, thank you.

Can I ask why you didn't commit the remaining two patches? I can accept if you devs decide they shouldn't go to the trunk, I'd just like to know the reason.

Most likely as Belugas doesn't know much about the oldloader for example. Which brings it to me, and I have a question about it: how does TTDp store moreanimations? As this always loads 256 entries... might be nice to make it also load TTDp games correctly

Hmm, reminds me, I should look into the TTDp loader again, I still want it to load more TTDp games correctly, even more now OTTD has most of the things too, in some way... sorry, offtopic

[Csaboka]

Most likely as Belugas doesn't know much about the oldloader for example.

Devs don't blindly commit code they can't understand. Makes sense Can I ask who is responsible for AnimateTiles & co., then?

Which brings it to me, and I have a question about it: how does TTDp store moreanimations? As this always loads 256 entries... might be nice to make it also load TTDp games correctly

First of all, the old 256-element animation array is still saved in the savegame, but its contents are bogus. The real animated tile list is saved as an extra TTDPatch chunk, with id=5. It doesn't have any special header; the chunk length will tell you the length of the animation array (ie. what moreanimation was set to when the game was saved). You can use the same logic for loading it as used for loading the old animation array, except that the size isn't fixed at 256 bytes.

Hmm, reminds me, I should look into the TTDp loader again, I still want it to load more TTDp games correctly, even more now OTTD has most of the things too, in some way... sorry, offtopic

Feel free to PM me about what the different chunk IDs mean and what their format is. I can certainly help with chunks I've added, and probably the others as well.