On Saturday, 23 March 2024 at 04:32:29 UTC, harakim wrote:
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.

Alright. I suppose this is largely what I'm doing.

Thank you very much for looking through my code and making the effort to understand it. So far you are the first person to comment on my code, beyond the little excerpts I've pasted directly onto this forum.

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.

When you said "tick system", I thought you meant that it would be asynchronous to the framerate, but then you say "At the end of the loop, you draw everything". Right now the function that moves units across the screen moves them each frame a distance proportional to the duration of the previous frame. I was thinking that a tick system would go hand-in-hand with making animations happen in a separate thread, but it sounds like you're talking about the same thread. Are you suggesting a fixed framerate?

Because this is a turn-based game, I don't need to update the factions and win conditions every frame, but at most every time a unit makes a move or when a new turn starts.

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.

That's nice. I thought it might be an amateurish solution to something that better programmers would do differently, similar to how I use `writeln` to print variables for debugging. It looks like I don't yet have any calls to this function, so perhaps I should add one under the debug configuration.

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 didn't immediately understand what you were saying, but then I looked at what line `unit.d:44` was at the time you wrote this.
```
map.getTile(xlocation, ylocation).setOccupant(this);
```
This is during the Unit's constructor, where it gives the `Tile` object a reference to itself. What exactly is wrong with this? Can memory addresses change when a constructor completes? I assumed that objects come into existence at the beginning of the constructor function.

Anyway, I can change this by calling `Unit.setLocation` after creating a new `Unit` object. That's unless if there's a particular reason why you think I should make this a function of the `Map` object.

I suppose the current system for setting up objects from a `JSONValue` is messy because much of the data applies to the base `Tile` & `Unit` objects, but then some of it needs to be in `VisibleTile` & `VisibleUnit` because it has sprite information.

I would also remove the unit from the map and then delete the unit rather than removing the unit from within the map class.

Sure. I'll change that. Even when I wrote the Unit destructor I was thinking that perhaps this was a bad way to implement this. I suppose I should just call the `Map.deleteUnit` whenever a unit should be deleted, right? I was also thinking of replacing `Map.deleteUnit` with a function that removes all null references from `this.allUnits` which can be called after destroying a unit, but unless if I need to hyper-optimize, that probably won't be any better than the function I have.

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.

I don't quite understand. Are you saying that I shouldn't have objects reference each-other, like how a `Unit` object has a reference to a `Tile` object called `currentTile`, and that tile object has a reference back to the unit called `occupant`? I see how bugs may happen if there's ever a case where the references aren't mutual, which is the reason for the `verifyEverything` function that you praised. But I imagine things getting more complicated if I can't determine a unit's current tile just by looking at a reference in it. Do you really think it would be better if I removed `Unit.currentTile`, and just used `Unit.xlocation` & `Unit.ylocation` as parameters for `Map.getTile` instead?

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'm not sure what you mean by "keep track of the units separately from the map". Do you mean to not rely on the `Map.allUnits` array?

I have considered doing a rewrite of the rendering system, in which all the rendering, and possibly input during game are handled by a new `Renderer` object, which may have it's own array with all the units in it.

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.

Is this a matter of not having all that JSON-related code cluttering the top of the file? Does putting too much of it in the objects themselves make them take up more memory? Or does it really make bugs less likely?

Before going with the current approach, I intended to have a module called `loadData` which would read JSON files and make objects out of them, but then I figured it was easier to have the JSON data read directly in object constructors so that private and protected variables can be set. I've thought about going back to this approach, but I haven't because I couldn't think of a reason to. If I do, perhaps they should be placed in the same modules as the objects being constructed, instead of being placed together in the `loadData` module.

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.

Yep. At this very moment I have two feature branches on my local copy. I'll have to figure out how to merge them when complete, but I did this for motivation-management, so that I don't need to finish one feature when I feel like working on another.

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.

This sounds a little like my idea of the `Renderer` object, in which the state of what's on screen would be updated by calling it's methods, but having a log of the UI wasn't what I had in mind.

That being said, I've thought about the possibility of having a log of events that can be used to rewind, replay, or possibly save the game. I agree that this should be further down the line. Even the ability to save a game is not very high on the priority list now. I wasn't thinking of logging *every* UI event though.

your game engine

Thank you for implying that I wrote a "game engine". That sounds like an accomplishment on my part. I'm not really sure which parts of my code would be referred to as the "engine" though.

Reply via email to