Add fog of war to the patch map #4462

ender-titan1 · 2023-09-10T15:34:39Z

Brief Description of What This PR Does

This PR hides unexplored patches and regions from players

Related Issues

Progress Checklist

Note: before starting this checklist the PR should be marked as non-draft.

PR author has checked that this PR works as intended and doesn't
break existing features:
https://wiki.revolutionarygamesstudio.com/wiki/Testing_Checklist
(this is important as to not waste the time of Thrive team
members reviewing this PR)
Initial code review passed (this and further items should not be checked by the PR author)
Functionality is confirmed working by another person (see above checklist link)
Final code review is passed and code conforms to the
styleguide.

Before merging all CI jobs should finish on this PR without errors, if
there are automatically detected style issues they should be fixed by
the PR author. Merging must follow our
styleguide.

ender-titan1 · 2023-09-12T16:47:34Z

The basic implementation of this has been completed, including a cheat to reveal the entire map. However, I'm still unsure about it aesthetically. Currently, the map looks like this:

I don't know whether or not to include region borders with regions that only contain unknown patches. If I leave the borders, players will know how big neighbouring regions are before actually entering them, however, without the borders the patches look very disjointed from the rest of the map.

hhyyrylainen · 2023-09-13T08:23:09Z

I'd leave out the region border, like as you said otherwise the player will see the size of the region without having explored it.

ender-titan1 · 2023-09-14T17:55:32Z

I think this is ready for review now

buckly90 · 2023-09-16T01:36:05Z

Seems to work perfectly well with no appearant errors. Excellent job!

I love the mystery that this provides, and the risk of entering an unknown patch to figure out what conditions are present makes for interesting gameplay.
I'm a little worried this might make it a bit more oppresive to new or casual players though...
We'll need to discuss this among the team to determine how to handle it in relation to difficulty and complexity.

In the mean time, I'd say this is good to go as is, though it may be helpful to implement a toggle for this feature.

GameDungeon · 2023-09-16T02:45:31Z

I was going to suggest a toggle in the new game settings as well. For something like this, I think that is pretty critical.

src/auto-evo/RunResults.cs

src/microbe_stage/Patch.cs

src/microbe_stage/editor/PatchMapDrawer.cs

hhyyrylainen · 2023-09-29T07:08:50Z

Regarding the problem of this not updating when loading a save on the map screen, I can't immediately tell what has gone wrong. Next I'd need to use a debugger to see what's the normal map update and rendering flow (using breakpoints) and then compare that to what happens when a save is loaded. I'm pretty busy so I'm hoping someone else can do that. I'll help out there as a last resort. My guess as to what is wrong is that the editor tab swap logic is slightly flawed when loading a save which hasn't been a problem before but now with these changes is a problem. That's where I'd start investigating if the method call sequence when normally swapping tabs and when loading a save match or not.

Co-authored-by: Henri Hyyryläinen <henri.hyyrylainen@gmail.com>

…nto fog-of-war

ender-titan1 · 2023-10-01T13:58:24Z

I have managed to fix that one edge case I mentioned, where now multiple connections between the same two regions will be accounted for when creating paths. However, the way I implemented it (and what looks to be the easiest way to do so) came with a side effect. Some of the links leading to unexplored regions will be repositioned and change sides when said region is explored. I don't know if that is ok to leave in or if should it be fixed, so I'm just putting it out here.

hhyyrylainen · 2023-10-02T06:39:29Z

I haven't tested this yet, but I think it should be a pretty important goal to ensure the links or whatever don't visually jump around a lot when the player is switching patches. Otherwise it might look weird to the player to see the world changing shape as they move around.

hhyyrylainen

I didn't understand the map drawer fully before this PR and I still don't fully know it, so here's basically as full review of this PR as I can do.

hhyyrylainen · 2023-10-02T12:27:35Z

src/microbe_stage/editor/PatchMapNode.cs

+        Visible = visibilityState != MapElementVisibility.Undiscovered;
+        questionMarkLabel.Visible = visibilityState == MapElementVisibility.Unexplored;


Shouldn't these two lines be in a method that can be called if the state needs to change? And for example the comparison against a specific enum value done in VisibilityState setter would also benefit from calling such a method to ensure that one of these places isn't accidentally updated when the other isn't. Doing that also requires the questionmarklabel to be nullable to ensure the method doesn't run if it is called before _Ready is called.

hhyyrylainen · 2023-10-02T12:28:16Z

src/general/base_stage/IEditorWithPatches.cs

@@ -6,4 +6,6 @@ public interface IEditorWithPatches : IEditor
    public Patch CurrentPatch { get; }

    public void OnCurrentPatchUpdated(Patch patch);
+
+    public void UpdateReportTabPatchSelector();


The name here is not changed yet.

hhyyrylainen · 2023-10-02T12:28:46Z

src/general/MainMenu.tscn

@@ -898,7 +898,6 @@ visible = false
 visible = false

 [node name="NewGameSettings" parent="." instance=ExtResource( 18 )]
-visible = false


Just commenting again to add this (and my previous comment) to my current review.

hhyyrylainen · 2023-10-02T12:30:12Z

src/general/NewGameSettings.tscn

@@ -29,6 +29,7 @@
 [sub_resource type="StyleBoxEmpty" id=1]

 [node name="NewGameSettings" type="Control"]
+visible = false


The diff still shows this line existing...

hhyyrylainen · 2023-10-02T12:34:34Z

src/microbe_stage/PatchRegion.cs

@@ -40,6 +41,9 @@ public enum RegionType
        Predefined = 3,
    }

+    [JsonProperty]
+    public MapElementVisibility VisibilityState { get; set; }


I see the existing code is not following the property order rule for JSON very well, but could you move this down to be after ID (but before Adjacent)? That'd make this change correct even if the older code was not correct...

hhyyrylainen · 2023-10-02T12:44:45Z

src/microbe_stage/editor/PatchMapDrawer.cs

@@ -443,6 +546,67 @@ private Vector2[] GetLeastIntersectingPath(PatchRegion start, PatchRegion end)
            probablePaths.Add((new[] { startCenter, intermediate1, intermediate2, endCenter }, -i));
        }

+        // Discard all paths intersecting patches in partially discovered regions
+        var pathsToDiscard = new List<Vector2[]>();


I think it would be worth it to make this into a field. That way the list can be reused for separate calls of GetLeastIntersectingPath to save on a lot of object allocations (just need to remember to Clear this list after the discarded paths are deleted).

hhyyrylainen · 2023-10-02T12:46:02Z

src/microbe_stage/editor/PatchMapDrawer.cs

+            }
+
+            if (IgnoreFogOfWar)
+                return;


Is return correct here or should this be continue instead?

hhyyrylainen · 2023-10-02T12:47:04Z

src/microbe_stage/editor/PatchMapDrawer.cs

@@ -753,12 +992,26 @@ private void RebuildMapNodes()
        foreach (var entry in Map.Patches)
        {
            var node = (PatchMapNode)nodeScene.Instance();
+
+            if (IgnoreFogOfWar)
+                entry.Value.Explored = true;


Is it required that the map drawer changes the state of the patch here? I really think that the map drawer shouldn't change the state of the map at all. Can this be safely removed? And potentially other code adjusted that doesn't know about ignoring the fog of war.

hhyyrylainen · 2023-10-02T12:48:44Z

src/microbe_stage/editor/PatchMapDrawer.cs

+    ///   A link between two <see cref="PatchRegion"/>.
+    ///   Used in <see cref="AdjustPathEndpoints"/>
+    /// </summary>
+    private class RegionLink


I have a slight feeling that this could be redone with a struct to save on memory allocations. Could you add a TODO remark here about investigating that?

hhyyrylainen · 2023-10-02T12:50:18Z

src/microbe_stage/editor/PatchMapDrawer.cs

+            Points = points;
+        }
+
+        public (Vector2[] Points, int Endpoint, int Intermediate, float Distance, Int2 Id) ToTuple()


What's the purpose of making a tuple out of this class? I see just using this to read a few variables. I think that just makes unnecessary data copies. I'd convert that code to just directly reading the fields of this class to avoid unnecessary complexity and data copies.

ender-titan1 · 2023-10-02T17:55:40Z

I'm kinda in the process of a big refactor to the drawer to solve a lot of issues I'm having, so I'll convert this to a draft until I think this new version is ready

As we only want to draw multiple lines if one of the regions is unexplored

as it was unnecessary

revolutionary-bot · 2023-10-27T21:48:09Z

We have recently refactored the entire microbe stage to use an Entity Component System architecture. Due to the extreme amount of work this conversion required, the change has already been merged to master. However, not all features are bug free or even work at all yet. Hopefully this does not cause too much disruption to other people's work, but some PRs depending on now broken functionality may need to wait. Also the CI checks may not pass even on master. Before that is fixed PRs that don't add new errors can be merged even with failing checks.

Any help with reimplementing / fixing the entire game back up again is extremely welcome. The next Thrive release is planned to be at the end of November 2023. Before that happen the game has to be at least close to as functional as before.

Note that the setup instructions have a new section on the native library which is required to be able to run Thrive at all in the future. Any improvement suggestions to the instructions or the compiling scripts are also welcome.

ender-titan1 added 6 commits September 10, 2023 11:00

Added basics for implementing fog of war

659a409

Added basic fog of war implemetation

9c6ce8d

Improved fog of war

75bb23c

Hide info from player when inspecting unknown patches

cc0dc3a

Finish basic implementation

4276a09

Add freebuild implementation

9eb100d

hhyyrylainen added this to In progress in Thrive Planning via automation Sep 10, 2023

ender-titan1 added 4 commits September 11, 2023 22:13

Stopped unexplored patches from highlighting connections

196b8a5

Fixed problem with changing region links

a0b87b7

Auto-evo exploring tool ignores fog of war

b3be32c

Add cheat to reveal the entire patch map

818fb0a

ender-titan1 added 8 commits September 13, 2023 21:59

Fixed issue with incorrectly rendered region links

3a9b820

Undo accidental change

bee8c09

Moved code to different editor class

79f8810

Fixed formatting

c995838

Merge master into fog-of-war

5dd669c

Update localization

aa0e079

Removed redundant field

f21193f

Removed manual setter

5f7f369

ender-titan1 marked this pull request as ready for review September 14, 2023 17:55

hhyyrylainen added the review label Sep 15, 2023

hhyyrylainen requested review from a team September 15, 2023 11:06

hhyyrylainen added this to the Release 0.6.4 milestone Sep 15, 2023

hhyyrylainen reviewed Sep 29, 2023

View reviewed changes

src/auto-evo/RunResults.cs Show resolved Hide resolved

hhyyrylainen reviewed Sep 29, 2023

View reviewed changes

src/microbe_stage/Patch.cs Outdated Show resolved Hide resolved

hhyyrylainen reviewed Sep 29, 2023

View reviewed changes

src/microbe_stage/editor/PatchMapDrawer.cs Show resolved Hide resolved

hhyyrylainen reviewed Sep 29, 2023

View reviewed changes

src/microbe_stage/editor/PatchMapDrawer.cs Outdated Show resolved Hide resolved

ender-titan1 and others added 7 commits September 29, 2023 20:17

Update RunResults.cs

2ff1190

Co-authored-by: Henri Hyyryläinen <henri.hyyrylainen@gmail.com>

Implement requested changes

294616d

Merge branch 'fog-of-war' of https://github.com/ender-titan1/Thrive i…

e391456

…nto fog-of-war

Get loading to work mostly properly

42ad335

Clean up some code

20c380e

Account for multiple connections between the same regions

d32ec81

Some more cleanup

bad76f7

hhyyrylainen requested changes Oct 2, 2023

View reviewed changes

ender-titan1 added 2 commits October 2, 2023 19:33

Begin drawer refactor

cf5e093

Update AdjustEndpoints method

0434c71

ender-titan1 marked this pull request as draft October 2, 2023 17:55

ender-titan1 added 7 commits October 2, 2023 20:05

Finish base for refactor

abf86ff

Make sure graphics are updated

4cf2a16

Stop rendering multiple links between explored regions

2d2dec0

As we only want to draw multiple lines if one of the regions is unexplored

RegionLink now handles endpoint adjustment

20e3e27

Add an algorithm to separate endpoints

5070256

Remove RegionLinkGroup

821cf1c

as it was unnecessary

Make sure multi-patch connections do not shitft

b93b86f

ender-titan1 closed this Oct 27, 2023

hhyyrylainen moved this from In progress to started but stuck (help wanted) in Thrive Planning Dec 8, 2023

ender-titan1 mentioned this pull request Dec 14, 2023

Add Fog Of War #4670

Merged

10 tasks

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add fog of war to the patch map #4462

Add fog of war to the patch map #4462

ender-titan1 commented Sep 10, 2023 •

edited by hhyyrylainen

ender-titan1 commented Sep 12, 2023

hhyyrylainen commented Sep 13, 2023

ender-titan1 commented Sep 14, 2023

buckly90 commented Sep 16, 2023

GameDungeon commented Sep 16, 2023

hhyyrylainen commented Sep 29, 2023

ender-titan1 commented Oct 1, 2023

hhyyrylainen commented Oct 2, 2023

hhyyrylainen left a comment

hhyyrylainen Oct 2, 2023

hhyyrylainen Oct 2, 2023

hhyyrylainen Oct 2, 2023

hhyyrylainen Oct 2, 2023

hhyyrylainen Oct 2, 2023

hhyyrylainen Oct 2, 2023

hhyyrylainen Oct 2, 2023

hhyyrylainen Oct 2, 2023

hhyyrylainen Oct 2, 2023

hhyyrylainen Oct 2, 2023

ender-titan1 commented Oct 2, 2023

revolutionary-bot commented Oct 27, 2023

		Visible = visibilityState != MapElementVisibility.Undiscovered;
		questionMarkLabel.Visible = visibilityState == MapElementVisibility.Unexplored;

Add fog of war to the patch map #4462

Add fog of war to the patch map #4462

Conversation

ender-titan1 commented Sep 10, 2023 • edited by hhyyrylainen

ender-titan1 commented Sep 12, 2023

hhyyrylainen commented Sep 13, 2023

ender-titan1 commented Sep 14, 2023

buckly90 commented Sep 16, 2023

GameDungeon commented Sep 16, 2023

hhyyrylainen commented Sep 29, 2023

ender-titan1 commented Oct 1, 2023

hhyyrylainen commented Oct 2, 2023

hhyyrylainen left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

ender-titan1 commented Oct 2, 2023

revolutionary-bot commented Oct 27, 2023

ender-titan1 commented Sep 10, 2023 •

edited by hhyyrylainen