Skip to content

Conversation

WildWeazel
Copy link
Member

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.

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 WildWeazel linked an issue Jun 15, 2022 that may be closed by this pull request
@WildWeazel WildWeazel marked this pull request as draft June 15, 2022 04:37
@WildWeazel WildWeazel changed the title Draft: 132 component event framework 132 component event framework Jun 15, 2022
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
@WildWeazel
Copy link
Member Author

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.

@WildWeazel
Copy link
Member Author

@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.

Copy link
Member

@QuintillusCFC QuintillusCFC left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

// END Production phase

gameData.turn++;
TurnEnded?.Invoke(null, null);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Not sure what the original tutorial was but this looks somewhat related https://learn.unity.com/project/65de084fedbc2a0699d68bfb

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Here's the archive of the wiki page: https://web.archive.org/web/20210304075110/https://wiki.unity3d.com/index.php/Toolbox
It's really just the concept of having a dumb Singleton container to hold unique instances of classes instead of making them all Singletons themselves. There may be a more Godot canonical way of achieving that part.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Let me clarify, the Unity and Godot references are irrelevant here; this is pure C# in the engine layer. Even the Unity implementation was derived from the original source on an IBM blog, now mirrored here: https://gurkantech.blogspot.com/2008/10/use-your-singletons-wisely_02.html
The core idea here is that being a Singleton has messy implications, so you make the Singleton itself as simple as possible and have it keep references to other arbitrary objects that you want to make globally accessible. I've seen similar implementations elsewhere: for example, Android's Context.getSystemService takes a class or name and returns the corresponding "service" object which is an arbitrarily typed resource that is unique to some scope but not a Singleton.

_gameData.turn = nextTurnNumber;
CurrentTurn = GetGameTurn(nextTurnNumber);

Console.WriteLine("Date is now " + CurrentTurn.TurnDate);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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...");
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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)

{
//Oh hai, we do need this handler here!
LowerRightInfoBox.SetTurn(turnNumber);
//LowerRightInfoBox.SetTurn(turnNumber);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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?

@QuintillusCFC
Copy link
Member

All the previous comments are stylistic/not functional issues. Now I've tested it in-game, and noticed a couple things:

  • If you go into a game, then go back to the main menu, and then go back into the game, you get an error: An item with the same key has already been added: Key: C7Engine.Components.CalendarComponent. We probably ought to clear the components out on going to the main menu.
  • When you start a game, it still says Turn 0, and then it becomes 4000 BC after the turn ends (and then 3950, etc.). It should start out at 4000 BC, and then go to 3950.

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".

@WildWeazel
Copy link
Member Author

Here is a slightly more involved example that's been kicking around in my head for a long time. I wrote it on the fly in a text editor and it assumes some features that don't yet exist, so consider it pseudocode but it illustrates how some actual game logic can be decoupled from the data it's working with:

class MilitaryLeaderComponent : GameRuleComponent
{
	public static String MGLCreatorTag = "MGL.Creator"
	public event Action MilitaryLeaderSpawned;
	
	Game Game = { get; set; };

	public MilitaryLeaderComponent(Game game)
	{
		this.Game = game;
		this.Game.Component<UnitComponent>().UnitCombatResolvedEvent += this.onUnitCombatVictory;
	}
	
	private void onUnitCombatVictory(UnitCombatResolvedEvent e)
	{
		UnitPrototype bcuType = e.Winner.Player.Faction.GetBattleCreatedUnit()
		if (e.Winner.ExperienceLevel == this.Game.Rules.XP.Ranks.Last() && !e.Winner.HasTag(MGLCreatorTag) && 
			this.Game.Rules.PlayerEligibleForUnit(e.Winner.Player, bcuType)) 
		// EligibleForUnit may check anything based on the unit's properties; in this case whether the player currently has any
		// GetBattleCreatedUnit could contain logic to vary by faction
		{
			// winner is eligible to create a MGL

			if (this.Game.Data.RNG.RollForOdds(this.Game.Rules.GetOddsOfMGL(e.Winner.Player.Faction)))
			// RollForOdds returns True 1 in X times
			// GetOddsOfMGL looks up rules on base rates, faction modifiers, etc
			{
				e.Winner.AddTag(MGLCreatorTag);

				// CreateUnit may raise more generic events internally
				// MGLType may have internal logic to name itself according to the faction's rules
				MapUnit newMGL = this.Game.Data.CreateUnit(bcuType, e.Winner.Player.Faction, e.Winner.Location);

				// UI layer receives this event, and if the player is human, displays a message and the rename unit prompt
				// GameRecordComponent receives this event and logs the data, location, and player for the game replay
				this.MilitaryLeaderSpawned?.Invoke(new MilitaryLeaderEvent(newMGL));
			}
		}
	}
}

Notice a few things here:

  1. The component is only concerned with whether to create a MGL, not the battle itself or the MGL once created (even the build army event need not be coupled here, because that ability might belong to a different unit type)
  2. It makes almost no assumptions about the other rules adjacent to MGLs, providing opportunity for mods to override some of these function calls to change those rules. Most of these functions would by default pass through to query the game data/rules, but could be intercepted to provide additional logic for different players, difficulties, etc.
  3. The use of a tag (just about the only hard-coded assumption, because it "owns" the tag) to indicate a one-off data point for the victorious unit

@WildWeazel
Copy link
Member Author

More notes...

Currently the ComponentManager registers components by their actual type. Consider either giving AddComponent an override to take an explicit key type OR giving GameComponent a function to return the desired key type which defaults to its own type. This would allow mock types to imitate their parent or other types in tests so as to be transparent to other components that depend on them. Would this be a code smell, and if so does it suggest that Singleton is the wrong tactic here (since the Singleton cannot practically be extended to modify its behavior for tests) and the ComponentManager should instead be accessible by widespread dependency injection?

GameComponent should have an Init() method, which ComponentManager calls after all components have been added. This allows for a two-step initialization so that components can arbitrarily depend on each other regardless of the order they are created in.

More GameComponent candidates:

  • Civ elimination (listen for city/unit loss events; raise elimination event)
  • Score (listen for many events; modify game data)
  • Auto save (fix up code to work now that we can save; raise save event (could be shown in UI)
  • Civ rankings popups (listen for score, turn events; raise UI event)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Component-Event Framework

3 participants