Oh boy programming debates, these are always fun!
Eight posts in and there's already various opposing justifications and viewpoints without any lead programmers even posting.
To stay on topic I'm just gonna address the points about the project, also note this is all just my opinion and I can't speak for Daiky and if I had done it today I probably would've done it completely differently and etc etc.
- Why are we not using YAML anchors and aliases instead of thesaveId() functions? The file structure will be cleaner, and so will the code.
Don't YAML nodes just point to others? You'd still have to lookup the equivalent data structure in SavedGame, so I dunno about making the code clearer, although I guess the file format would be. But I don't know what yaml-cpp's support for them is like, there's no code examples.
- Instead of creating the objects and then loading them, why don't we have a "loading" constructor? Again, the code will be cleaner, since we can remove some functions that only get used once.
Consistency I guess. Not every object is only loaded on construction (eg. modular rulesets), and it leaves the objects more open for alternate loading later.
- Why don't we have the Game object as a global object (or Singleton)? It is required in LOTS of places and we just store and pass it around (or it's Ruleset and SavedGame members, which amounts to the same thing). Making it visible everywhere would simplify LOTS of code and remove redundant arguments from lots of functions. It's not like we will ever have more than one.
Singletons are... debatable. I don't really like them and I've always been taught not to like them. I don't see the need to arbitrarily restrict a class.
In this case you just want it as a global so you don't have to pass around pointers, but I don't think that really fixes the problem. Most game entities shouldn't need or even
care about the entire game state, they just need particular bits. Game just started off as an abstraction for the core engine stuff but kinda grew into this big encompassing class because well, stuff has to go somewhere, and as particular elements needed particular stuff it just went all over the place. I guess optimally Game should be broken up so elements only need to reach what they need or something? I don't know how to properly redesign it so this wouldn't happen, OOP is great in theory until you find yourself not knowing how to actually put it into practice outside isolated examples, and you can end up going mad debating such design into infinity if you don't move on.
- Since we gave get/set for every member in some classes, why not make them plain structs?
Whether it's better to have public fields or getter/setters is arguable. Personally I just do it for the consistency, if I do something once I do it all the time, and it makes fields easier to control and document (specially since OpenXcom is ever-changing). Yeah it becomes a lot of boilerplate code in some classes, but there's plenty of tools to generate it for you.
- Why do we return pointers to collections (eg Game::getUfos()) instead of references? The code has a lot of places where references are more suited to what we mean instead of pointers (eg Everywhere we store Game, Rule* and SavedGame pointers in classes as fields).
Again, consistency. Probably has to do with old habits, I've always worked with pointers, so when references popped up I never got into them. What is their advantage over pointers? Aside from hiding the * which I don't really mind.
- Why do we return non-const rules? They should not have writable fields after they are initialized.
Aren't rules read-only already? Anyways const-correctness is one of those things that takes forever to implement and forever to master. I don't know everywhere where it should go, though I've done my best to apply it across OpenXcom without breaking anything, so feel free to add more if you want.
Classes having their data blindly manipulated from others directly is not OOP.
OOP means having objects represent ideas and keeping interactions to a minimum, each object containing all the data it needs and preneting the actions that make sense. So, for example, the Ufo should present methods for moving, damaging it or checking for interesting conditions (destroyed, landed, moving). Not give free access to everyone to directly set member variables. Moving all moving things on the map should be the responsibility of the class holding the list of all movables on the map, etc for many things.
There's always a new evil going around in programming circles. Back in my day, getters and setters were all the rage.
Seriously though, I've seen this argument go around a lot with OpenXcom. "Oh no, your code is just getters/setters instead of
proper abstractions, the horror!". I'll admit I'm not an amazing coder and OpenXcom isn't the pinnacle of design. But have you tried implementing game rules, especially for strategy stuff like X-Com? You'll be shocked to know most of it
is just getting/setting values, there's no big deep simulation or behavior or whatever, and trying to fully abstract it all away results in design nightmares because game rules aren't all nice and well-behaved, they're tons of values interacting with each other.
For example, take a Craft. You think "oh a craft has to move to places". So you need to change its position, just add a "setPosition". Then you think "but but but the craft should move to places, not be directly controlled". Ok you instead put a "setDestination" (maybe you'd prefer calling it "moveTo", it's just semantics), it does what it says, you give it a destination and the Craft slowly goes there by its rules. Woo, OOP, etc. But wait what about when you need to move the craft instantly, like when it's transferred between bases? Whoops turns out you needed that direct position control after all! And you'll run into this kind of stuff
all the time, and your wonderful design goes to pot as you need to get stuff done and players just want to play things.
Our Game object now has 3 distinct roles. It's the main loop, the game state database and the rules database.
The main loop is a service and should be a global object.
Many things require the ruleset, and we only ever care for one, so a global ruleset is not unreasonable.
Since our objects don't have a clean state separation (data and behaviour are split across many objects), we need to pass the savegame around. But that is because it too has two roles. It's the 'persistent data' and also the 'current game state of all objects'. The 'current game state' is also a candidate for a global object, to be used everywhere except load/save routines (which actually care for the persistent data).
I'm not sure I understand your definitions here, so this is the way I see the OpenXcom design:
- The Game class handles the core engine stuff, the SDL subsystems, the state and event management. It also contains the game contents (resource, ruleset, save) out of convenience.
- The states take care of all the game behavior and interaction with the player, and are what connect and interact with everything else forming a running game.
- The SavedGame and related classes are intended to contain the variable game state, everything that needs to be saved to disk to preserve the game. The 'current game state of all objects'
is the 'persistent data', so serializing Savegame saves the game and preserves the game state. This is ignoring superficial stuff which you might consider "state" like UI etc. Originally it was just intended to be "dumb" variable storage that the states would manipulate, but some behavior ended up leaking into it, so now it's kinda mixed between states and savegames.
- The Ruleset and related classes are intended to contain the fixed game variables. This is all the fixed definitions in the game rules, all the same fixed values from game to game. No behavior.
Hope that clears things up.
