Skip to content

Commit

Permalink
Add the worker "mine" action and fix UI updates after moves
Browse files Browse the repository at this point in the history
This PR has two parts, and I honestly would normally split them into two
(or even 3) PRs but git doesn't make this very easy (I miss my `hg csplit`).

The first part is straightforward: just mirroring all the existing logic
for building roads, but with method names for mines instead. Things like
the logic for whether we can build a mine, the keyboard shortcut, the
message to the engine, etc. This required adding logic for whether a
tile has a volcano, since that's a special tile for mines.

The second part was a bit trickier: ensuring that the UI was correctly
updated after moving a unit, when that unit still had movement points
available. This was partially implemented for keyboard-based moves in
`Game.cs`, but I suspect this was fundamentally racy - it raced the
message to the engine being received and updating the unit's position
against setting the selected unit. I tried duplicating this for the goto
logic and found it racy, so I suspect the same was true for the keyboard
moves, but I didn't exhaustively test this.

The solution here is to add a new message back to the UI from the
engine. Once the engine has finished doing all of its work on updating
the position of the unit we can safely reset the selected unit without
fear of races. This ensures that if a unit walks from a roaded tile
with a mine onto a roaded tile that doesn't have a mine, the mine button
appears (for workers). It also fixes updating the move counter in the
lower right hand info panel.
TomWerner committed Jan 17, 2025
1 parent 41a6adf commit 03ad11b
Showing 13 changed files with 90 additions and 2 deletions.
15 changes: 14 additions & 1 deletion C7/Game.cs
Original file line number Diff line number Diff line change
@@ -183,6 +183,16 @@ public void processEngineMessages(GameData gameData) {
}
}
break;
case MsgUpdateUiAfterMove mUUAM:
// The unit finished moving and still has moves left, so we need to
// mark it as the selected unit again.
//
// Among other things, this will refresh the UI and ensure that the
// unit action buttons are correct.
if (CurrentlySelectedUnit != MapUnit.NONE) {
setSelectedUnit(CurrentlySelectedUnit);
}
break;
}
}
}
@@ -526,7 +536,6 @@ private void ProcessAction(string currentAction) {

if (dir.HasValue) {
new MsgMoveUnit(CurrentlySelectedUnit.id, dir.Value).send();
setSelectedUnit(CurrentlySelectedUnit); //also triggers updating the lower-left info box
}
}

@@ -608,6 +617,10 @@ private void ProcessAction(string currentAction) {
new MsgBuildRoad(CurrentlySelectedUnit.id).send();
}

if (currentAction == C7Action.UnitBuildMine && CurrentlySelectedUnit.canBuildMine()) {
new MsgBuildMine(CurrentlySelectedUnit.id).send();
}

}

private void GetNextAutoselectedUnit(GameData gameData) {
1 change: 1 addition & 0 deletions C7/Text/c7-static-map-save.json
Original file line number Diff line number Diff line change
@@ -58936,6 +58936,7 @@
],
"actions": [
"unit_build_road",
"unit_build_mine",
"unit_hold",
"unit_wait",
"unit_fortify",
2 changes: 1 addition & 1 deletion C7/UIElements/UnitButtons/UnitButtons.cs
Original file line number Diff line number Diff line change
@@ -61,7 +61,7 @@ public override void _Ready() {

AddNewButton(specializedControls, new UnitControlButton("fortress", 0, 3, onButtonPressed));
AddNewButton(specializedControls, new UnitControlButton("barricade", 4, 4, onButtonPressed));
AddNewButton(specializedControls, new UnitControlButton("mine", 1, 3, onButtonPressed));
AddNewButton(specializedControls, new UnitControlButton(C7Action.UnitBuildMine, 1, 3, onButtonPressed));
AddNewButton(specializedControls, new UnitControlButton("irrigate", 2, 3, onButtonPressed));
AddNewButton(specializedControls, new UnitControlButton("chopForest", 3, 3, onButtonPressed));
AddNewButton(specializedControls, new UnitControlButton("chopJungle", 4, 3, onButtonPressed));
5 changes: 5 additions & 0 deletions C7/project.godot
Original file line number Diff line number Diff line change
@@ -183,6 +183,11 @@ unit_build_road={
"events": [Object(InputEventKey,"resource_local_to_scene":false,"resource_name":"","device":-1,"window_id":0,"alt_pressed":false,"shift_pressed":false,"ctrl_pressed":false,"meta_pressed":false,"pressed":false,"keycode":0,"physical_keycode":82,"key_label":0,"unicode":114,"echo":false,"script":null)
]
}
unit_build_mine={
"deadzone": 0.5,
"events": [Object(InputEventKey,"resource_local_to_scene":false,"resource_name":"","device":-1,"window_id":0,"alt_pressed":false,"shift_pressed":false,"ctrl_pressed":false,"meta_pressed":false,"pressed":false,"keycode":0,"physical_keycode":77,"key_label":0,"unicode":109,"echo":false,"script":null)
]
}

[mono]

4 changes: 4 additions & 0 deletions C7Engine/EntryPoints/CityInteractions.cs
Original file line number Diff line number Diff line change
@@ -19,7 +19,11 @@ public static void BuildCity(int x, int y, ID playerID, string name) {
gameData.cities.Add(newCity);
owner.cities.Add(newCity);
tileWithNewCity.cityAtTile = newCity;

// Cities are treated as though they have a road, but if
// a city is build on a mine, the mine should be removed.
tileWithNewCity.overlays.road = true;
tileWithNewCity.overlays.mine = false;
}

public static void DestroyCity(int x, int y) {
25 changes: 25 additions & 0 deletions C7Engine/EntryPoints/MessageToEngine.cs
Original file line number Diff line number Diff line change
@@ -56,6 +56,12 @@ public MsgMoveUnit(ID unitID, TileDirection dir) {
public override void process() {
MapUnit unit = EngineStorage.gameData.GetUnit(unitID);
unit?.move(dir);

// The unit moved to a new tile - if it still has movement points,
// update the UI to reflect this new position and movement points.
if (unit?.movementPoints.canMove == true) {
new MsgUpdateUiAfterMove().send();
}
}
}

@@ -73,6 +79,12 @@ public MsgSetUnitPath(ID unitID, Tile tile) {
public override void process() {
MapUnit unit = EngineStorage.gameData.GetUnit(unitID);
unit?.setUnitPath(EngineStorage.gameData.map.tileAt(destX, destY));

// The unit moved to a new tile - if it still has movement points,
// update the UI to reflect this new position and movement points.
if (unit?.movementPoints.canMove == true) {
new MsgUpdateUiAfterMove().send();
}
}
}

@@ -130,6 +142,19 @@ public override void process() {
}
}

public class MsgBuildMine : MessageToEngine {
private ID unitID;

public MsgBuildMine(ID unitID) {
this.unitID = unitID;
}

public override void process() {
MapUnit unit = EngineStorage.gameData.GetUnit(unitID);
unit?.buildMine();
}
}

public class MsgChooseProduction : MessageToEngine {
private ID cityID;
private string producibleName;
1 change: 1 addition & 0 deletions C7Engine/EntryPoints/MessageToUI.cs
Original file line number Diff line number Diff line change
@@ -38,6 +38,7 @@ public MsgStartEffectAnimation(Tile tile, AnimatedEffect effect, AutoResetEvent

public class MsgStartTurn : MessageToUI { }

public class MsgUpdateUiAfterMove : MessageToUI { }

public class MsgCityDestroyed : MessageToUI {
public City city;
4 changes: 4 additions & 0 deletions C7Engine/EntryPoints/UnitInteractions.cs
Original file line number Diff line number Diff line change
@@ -54,6 +54,10 @@ public static List<string> GetAvailableActions(MapUnit unit) {
result.Add(C7Action.UnitBuildRoad);
}

if (unit.canBuildMine()) {
result.Add(C7Action.UnitBuildMine);
}

// Eventually we will have advanced actions too, whose availability will rely on their base actions' availability.
// unit.availableActions.Add("rename");

22 changes: 22 additions & 0 deletions C7Engine/MapUnitExtensions.cs
Original file line number Diff line number Diff line change
@@ -452,5 +452,27 @@ public static void buildRoad(this MapUnit unit) {
unit.movementPoints.onConsumeAll();
}

public static bool canBuildMine(this MapUnit unit) {
// Mines can only be built on land, if there is no mine already there,
// and if there isn't a city.
//
// Volcanos also cannot be mined.
return unit.unitType.actions.Contains(C7Action.UnitBuildMine) &&
unit.location.IsLand() &&
!unit.location.IsVolcano() &&
!unit.location.overlays.mine &&
unit.location.cityAtTile == null;
}

public static void buildMine(this MapUnit unit) {
if (!unit.canBuildMine()) {
log.Warning($"can't build mine by {unit}");
return;
}

// TODO add animation and long process of building
unit.location.overlays.mine = true;
unit.movementPoints.onConsumeAll();
}
}
}
2 changes: 2 additions & 0 deletions C7GameData/Actions.cs
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
namespace C7GameData {
// The strings for each action correspond to values in project.godot for keyboard shortcuts
public static class C7Action {
public const string EndTurn = "end_turn";
public const string Escape = "escape";
@@ -16,6 +17,7 @@ public static class C7Action {
public const string UnitBombard = "unit_bombard";
public const string UnitBuildCity = "unit_build_city";
public const string UnitBuildRoad = "unit_build_road";
public const string UnitBuildMine = "unit_build_mine";
public const string UnitDisband = "unit_disband";
public const string UnitExplore = "unit_explore";
public const string UnitFortify = "unit_fortify";
3 changes: 3 additions & 0 deletions C7GameData/ImportCiv3.cs
Original file line number Diff line number Diff line change
@@ -464,6 +464,9 @@ private void ImportUnitPrototypes() {
if (prto.BuildRoad) {
prototype.actions.Add(C7Action.UnitBuildRoad);
}
if (prto.BuildMine) {
prototype.actions.Add(C7Action.UnitBuildMine);
}
if (prto.Bombard) {
prototype.actions.Add(C7Action.UnitBombard);
}
4 changes: 4 additions & 0 deletions C7GameData/TerrainType.cs
Original file line number Diff line number Diff line change
@@ -27,6 +27,10 @@ public bool isHilly() {
return false;
}

public bool isVolcano() {
return Key.Equals("volcano");
}

//TODO: Once we have IDs, this should *not* rely on the display name.
//That will be after issue 58, which will be after PR 70.
public bool isWater() {
4 changes: 4 additions & 0 deletions C7GameData/Tile.cs
Original file line number Diff line number Diff line change
@@ -136,6 +136,10 @@ public bool IsAllowCities() {
return overlayTerrainType.allowCities;
}

public bool IsVolcano() {
return overlayTerrainType.isVolcano();
}

public TileDirection directionTo(Tile other) {
// TODO: Consider edge wrapping, the direction should point along the shortest path as the crow flies.

0 comments on commit 03ad11b

Please sign in to comment.