Reworking the control flow for my tactical role-playing game

[harakim]

I am not an expert but I would second the tick system. That is 
pretty solid advice. Just iterate through your events or, in your 
case, units and such objects and update them all. Then draw. Then 
go to the next tick.

You should engineer the system in a way that makes sense to you. 
The currency of finishing programs is motivation and that comes 
from success and believing you will succeed. If you're 
implementing XYZ pattern from someone else, if you don't get it 
then you will get unmotivated. I have seen some pretty horrible 
programs make it to all the way done and have almost never seen 
someone's first run at a problem be both pretty and get finished. 
Don't beat yourself up about keeping it clean. If you feel like 
it or it's getting out of control where you can't understand it, 
then you should go look for a solution. The motivation is in 
moving forward, though. This is probably the piece of advice I 
wish I had in the beginning! You can always refactor later when 
it's done.

Solicited advice:
* Move to the tick system. You make a loop and update each object 
based on the time that has passed. If your tick is 1/60th of a 
second then you make a loop. In the loop, you update the map 
state knowing 1/60th of a second has passed. Then you update all 
the items and move them or deplete them or whatever however much 
would happen in 1/60th of a second. Then you update all of the 
units. Then you can update the factions or check win conditions. 
At the end of the loop, you draw everything. (maybe raylib just 
draws whenever it wants, which is fine) In order to know what to 
update, you will have to save what action they are doing. ex. an 
arrow is flying toward a target. you will need to keep track of 
what it is doing so when you iterate, you can continue that train 
of thought. This will require a bit of a rewrite but I have 
worked on a game server that was used by thousands of people for 
years and it was based on this simple system. It scales really 
well and is pretty easy to understand.

Unsolicited advice:
* Your verifyEverything method is awesome. I call that strategy 
fail-fast. It means you fail right away when you have a chance to 
identify what went wrong.

* Construct the unit and then call map.setOccupant(unit) after 
the unit is constructed. I would not do anything complicated in a 
constructor. It's also generally frowned upon to pass a reference 
to an object to anything before the constructor completes. Most 
of the changes I mention are things to think about, but this 
specifically is something you ought to change. I would also 
remove the unit from the map and then delete the unit rather than 
removing the unit from within the map class.
unit.d:44 map.getTile(xlocation, ylocation).setOccupant(this);

* Another reason I would switch that line is that it's best to 
avoid circular dependencies where you can. It will make it hard 
to reason about either in isolation. It relates to that line 
because your map has units in it and your unit takes Map in the 
constructor. That is a red flag that you are too coupled. That 
concept is not a rule but just something to think about when you 
get stuck. This comment points to a symptom of the circular 
dependency: //Always set `destroy` to false when calling from the 
Unit destructor, to avoid an infinite loop.

* In your game loop, I would keep track of the units separately 
from the map, if you can. Go through the map and do updates and 
go through the units and update each one. If the logic is too 
tied together, don't worry about it for now.

* I would break the json loading into separate classes (eg 
FactionLoader, Unit loader) instead of being included in the map 
and unit class. I like to have code to intialize my programs 
separate so I don't have to look at it or think about it or worry 
about breaking it when working on my main code.

* You said
//Change this later so that the faction with the first turn is 
determined by the map file.
Comments like that are perfect. Jot down all your ideas while 
you're working on the main functionality. Once you have something 
working, tweaking it will be so much fun. Take side quests when 
you want to stay motivated, but I would stray away from the 
bigger ones until you have the basic functionality working. It's 
often the fastest way to get the side quest done anyway since you 
can test it.

* You should probably not do this, but it might give you some 
ideas for later. What I would do is make a separate thread for 
managing the UI state and push events to that thread through the 
mailbox. I have done this once (on my third version of a program) 
and it was by far the cleanest and something I was proud of. The 
benefit is you can publish those same events to a log and if 
something in your UI goes wrong, you can look at the log.

Better than that is the ability to replay your log. Instead of 
sending the events from a game engine, you have a module that 
just reads from the file and sends the events. Then you can debug 
where it went awry. And you can have a feature for players to do 
that to rewatch their game. You can also replace your game engine 
with a module that reads from the network for a multi-player game 
and it uses the exact same UI logic. In that case, you can save 
the network traffic in a log the same way to replay to diagnose 
network packet processing errors.

That method does not require any synchronization or any thread 
complexity because the communications are one way. However, you 
will not need it so I would put thinking about this and doing it 
on the list after everything that is motivating for you.