132 component event framework #277
Conversation
Move ComponentManager to the C7Engine project and namespace. Implement the CalendarComponent responsible for updating turn number and date. For now it is triggered by an event from TurnHandling and logs the date to console.
…efactor game scene and init sequence to better manage data and callbacks.
WildWeazel
changed the title
Moved ComponentManager and components into new folder and namespace, gave GameComponent an Initialize method, and allowed chaining ComponentManager method calls
Created a stub implementation for AutosaveComponent, reacting to TurnStarted events by generating an autosave filename
|
Added a stubbed-out AutosaveComponent, so you can start to see how the components can loosely interact with each other. ComponentManager is now a little more manage-able with method chaining, since presumably all (default) components will be added and initialized in one go. |
|
@QuintillusCFC and @maxpetul is this helpful so far? I want to at least refine the Node integration before merging but hopefully this gives you an idea of how the components can work together. There's not much in the way of events yet since these don't do much. I was thinking about refactoring unit combat into a component since there's a lot more going on there, but after looking at it I'm not sure how much it should be component-ized. It kind of makes sense for unit objects to act on each other directly, unlike other more abstract mechanics like governments or research. |
There was a problem hiding this comment.
This definitely helps illustrate the event/component paradigm as it relates to C7 for me. (I'm also bad at going out and reading documentation unless I need it right now, and learn best by example) I can definitely see how this lends itself to extensibility, and the calendar is a nice example. Now I know why you suggested a RandomComponent in the other PR.
| @@ -130,8 +131,7 @@ internal static void AdvanceTurn() | |||
| } | |||
|
|
|||
| // END Production phase | |||
|
|
|||
| gameData.turn++; | |||
| TurnEnded?.Invoke(null, null); | |||
There was a problem hiding this comment.
Is there a reason the sender argument is null here, but this in the other event invocations?
| using System.Collections.Generic; | ||
| using System.Linq; | ||
|
|
||
| // modeled after the Unity Toolbox pattern from https://wiki.unity3d.com/index.php/Toolbox |
There was a problem hiding this comment.
It looks like the Unity 3D Wiki moved, or maybe is down; when I try to go to that site I get wiki.unity3d.com’s server IP address could not be found.
| _gameData.turn = nextTurnNumber; | ||
| CurrentTurn = GetGameTurn(nextTurnNumber); | ||
|
|
||
| Console.WriteLine("Date is now " + CurrentTurn.TurnDate); |
There was a problem hiding this comment.
Probably ought to switch these to Serilogs now that we have that in the engine.
| date = String.Concat(date.Where(c => !char.IsWhiteSpace(c))); | ||
| date = String.Join("_", date.Split(Path.GetInvalidFileNameChars(), StringSplitOptions.RemoveEmptyEntries)); | ||
| string filename = $"auto_{date}.json"; | ||
| Console.WriteLine($"I would save {filename} right now if I knew how..."); |
There was a problem hiding this comment.
In part that's part of card #223 , in part it would use SaveFormat.Save in the C7GameData project. But I could also see having a component for saving/loading (not just autosaving...), and that's really slightly beyond this proof-of-concept.
(Also slightly beyond, but it occurred to me that this method, or at least the file name creation part of it, would lend itself nicely to some self-documenting units tests)
| @@ -33,7 +38,7 @@ private void OnTurnEnded() | |||
| private void OnTurnStarted(int turnNumber) | |||
| { | |||
| //Oh hai, we do need this handler here! | |||
| LowerRightInfoBox.SetTurn(turnNumber); | |||
| //LowerRightInfoBox.SetTurn(turnNumber); | |||
There was a problem hiding this comment.
Probably can take this method out now, I think... or maybe we should leave it for now since there are other things that will need updated as we add more features?
|
All the previous comments are stylistic/not functional issues. Now I've tested it in-game, and noticed a couple things:
Architecturally, the one caveat I'd note is that the front-end C7 project, specifically GameStatus.cs, is listening directly to the TurnStarted event. This is fine in single player, but I think it's a good example of how multi-player is tougher. Maybe there's still a TurnStarted event in MP that gets invoked in a different path? There are plenty of MP-related problems with the old way of doing things too. I'm definitely not saying it needs to be changed, it's more "if you/someone else on the team really wants MP, this might be the time to figure out how it would work". |
This will resolve #132
There's still some work to do especially for events but I wanted to get this much up for reference. I've implemented one of the simplest components, the calendar: it just updates the game turn and calculates the corresponding date.
Moved ComponentManager to C7Engine as it is part of the core code, and implemented CalendarComponent. Added component initialization in CreateGame, and replaced the turn increment in TurnHandling with an event for the calendar to subscribe to. Added a function in GameStatus to connect the component's event to the info box update. and call it just before the game starts.
The latter has to be done outside of _Ready for now because of the order of events in Godot: a node isn't ready until after all its children are ready, so _Ready propagates up the tree and thus Game initialization has not happened yet when UI elements become ready. I tried moving some code from Game._Ready to Game._EnterTree but immediately ran into problems so I went with a follow-up call into the child node. This is a common pattern as parents should be responsible for configuring their children, but I'm not necessarily happy with this solution in particular. That is probably a larger architectural issue to think about game-wide.
You'll also notice that I passed the game data reference into the component constructor even though it's a static object. We should avoid relying on static data (both dependency injection and the component model will help with that when used carefully), because it makes it very hard to track state and to test classes in isolation. That's another thing to address at a higher level, but here's a small counter-example.
So this is all just a first cut of everything, especially the Godot portion, but shows how the components can interact with the rest of the code. I still want to show more events, and maybe a second component like autosave so they can interact with each other.