Transport Tycoon Forums

Hi all, I'm new to the forum and new to OpenTTD. I've been studying the code for a few weeks and I noticed that a lot of game features are compiled directly into the game loop, with no way to turn them off or provide alternatives without lots of configuration flags or recompiling. For example, there is DisasterDailyLoop which runs on every new day, even if disasters are disabled. It checks _settings_game.difficulty.disasters, which is easy enough to change, but aside from changing that flag (or multiple flags) or installing a patch that changes disaster behavior, it is always tied into the daily loop. A better example is the code that marks town views dirty in case there was a snowline change, regardless what scenario you are running. To me that is a big warning sign.

I thought I would try to make the loops more abstract by refactoring some of the loop functions into an array of function pointers. Here's my current diff, which shouldn't change any game behavior at all. I'm using OSX, so the build/configure changes may not work for other systems, daily.h and daily.cpp are new files that need to get compiled in.

Feedback/comments/suggestions? There is plenty to do here, I am still tinkering.

Improvements to game speed are always welcome.

Do you have any testing data which prove one way or the other being advantegous?

_________________

OpenTTD: manual | online content | translations | Wanted contributions and patches
#openttdcoop: blog | wiki | public server | DevZone
DevZone - home of the free NewGRFs: OpenSFX | OpenMSX | OpenGFX | Swedish Rails | OpenGFX+ Trains|RV|Industries|Airports|Landscape | NML

This is very unlikely to be faster (instruction-wise). On the other hand, it also won't be much slower. But I also don't think that is his point, to make the game faster. More to make it more readable / sane.

What I like about this idea is, that it makes everything a bit more dynamic. It needs a few more refactoring to be more C++ (abstract class which gets added, which has a member which does the gameloop, for example, or even where you can specify DailyLoop, MontlyLoop, etc). It might just make the code more readable. But I am not sure yet what I like more.

Regarding your patch itself, there are some silly statements. First, _daily_calls for sure can be a dynamic array (a *_daily_calls, as you called it). But as we are programming in C++ here, it is much better to use a std::list (or any of his friends, I am guessing a std::vector would do? Not sure). There is no need to do this the C way.

Also, if you would be to go C++ style with classes, you can make the ctor automatically register itself to the dailyloop if needed, avoiding the AddDailyCall calls

Just my 2 penny.

_________________
The only thing necessary for the triumph of evil is for good men to do nothing.

I'm new to OpenTTD development as well, but I had a quick look at your diff file. It seems that you initialize your daily callbacks in the main function, i.e. at program start. This defeats the purpose as all callbacks are immediately added, even the conditional ones like the disaster loop you mentioned. Your implementation provides no way of removing a previously added callback, so there is no way of reacting to a change, e.g. when disasters are turned on or off in the difficulty settings. For what you intend to do, you would have to observe changes to the disaster setting and add/remove the callback accordingly.

This would reduce branching in the daily loop, which is always a good thing. However, it is countered by an additional for loop (which the compiler cannot inline because _daily_calls_count is unknown) and additional dereferencing. It might still be beneficial to performance, you'd have to test that, but it's probably not worth the hooking into settings change.

As a side note, the order of commands in DisasterDailyLoop() seems strange. Shouldn't _settings_game.difficulty.disasters be checked first? This saves the pointless decrement operation if nothing else.

Why not have all events (daily, monthly and yearly) pre-planned at the start of the year. You'd end up with a linked list of items to do which can simply be iterated through. At the end of the list an event could be attached to fill the new year. That way you don't need to figure out what needs to happen each day or whether or not a disaster occurs today, because that is already known.

Give it a shot. It gives a compiler error, performing arithmetic on a function pointer is not allowed. I wrestled with this quite a bit, and though there is probably some truly horrible way to force it, it would be easier in the long run to wrap all the function calls inside objects.

Even though the callbacks are set in the main loop, that doesn't prevent them from being subsequently changed - just reset the array and fill it up with new callbacks. I didn't do it here, it is just a demo, but it is now theoretically possible where it wasn't before.

Throughout the code of OpenTTD it is used many times, so the problem is at your end

typedef void (*LoopFP)();
LoopFP *_daily_calls;

void Test() {
  printf("Testn");
}

void main() {
  _daily_calls = malloc(sizeof(LoopFP) * 12);
  memset(_daily_calls, 0, sizeof(LoopFP) * 12);
  _daily_calls[0] = &Test;
  for (i = 0; i < 12; i++) {
    printf("%pn", _daily_calls[i]);
    if (_daily_calls[i] != NULL) _daily_calls[i]();
  }
}

This will most likely compile and work. Sometimes I love being a C programmer.

That said, this will not be accepted in OpenTTD any more, as we are C++ now. Using a std:: variant will be mandatory in these cases.

_________________
The only thing necessary for the triumph of evil is for good men to do nothing.

_daily_calls = malloc(sizeof(LoopFP) * 12) says: invalid conversion from ‘void*’ to ‘void (**)()

Care to try again?

A linked list would contain mostly nothing and waste unnecessary memory. What would work is a std::map<Date, IDateEvent> which can be used to schedule events for certain days. IDateEvent would define an execute() method which is called by the day loop on the appropriate day, and could be implemented e.g. by a Disaster class. To enable multiple events on the same day, defining IDateEvent as a composite pattern would work, or using a std::map<Date, std::vector<IDateEvent> >.

This is just software theory though, I lack the knowledge of OpenTTD architecture to say if it's actually applicable and useful.

Lol. Tone down the attitude, is my first advise. And like said: this is C code. Not C++. C++ needs a cast. But that you could have figured out yourself.

_________________
The only thing necessary for the triumph of evil is for good men to do nothing.

Here is a revised version of the patch, which wraps each function calls in its own functionoid class. This allows you to pull out some of dependencies in date.cpp, since only the classes need to know where to find those symbols, and those classes can live somewhere else (here they live in daily.*).

This only modifies OnNewDay, but the same technique would apply to OnNewMonth and OnNewYear. StandardDay() is used to initialize the current set of daily actions, but in theory any module could manipulate the daily loop.

Have you ever wanted to advance just one tick while the game is paused? Or advance several ticks without having to wait for the screen to draw each frame? Here's a little console function that will do that. Experimental, of course. Compile at your own risk.


Limited to a max of 99 ticks.

] x_tick 
] x_tick 1
] x_tick 99

The trees are just a problem. When you try to plant trees they are random after the first one. And the loop to grow tree tiles and the other loop that spawns new tree tiles don't have any configuration options, they are either on or off. But the devs aren't interested.

Until then, if you are brave enough to patch and compile, here is another console command, x_destroy_all_trees. It destroys all trees currently on the map. You can run it from the main menu, or while playing a game, or while editing a scenario. You could even load a saved game, run it, and resave the game.


This also includes the x_tick command from the previous post.