Implement a way to edit the cell division order in multicellular #4499

Kashnox · 2023-10-01T02:32:32Z

Brief Description of What This PR Does

This PR gives users the ability to change the order their cells are created during the early multicellular stage.

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.

… early multicellular species.

…e red, and blocking the user from closing the editor while they exist. Also started showing the reproduction order on the cells while the Reproduction tab is open.

hhyyrylainen · 2023-10-09T06:46:47Z

src/general/HexLayout.cs

+    /// </summary>
+    /// <param name="index1">The index of one of the items that should be swapped.</param>
+    /// <param name="index2">The index of the other item that should be swapped.</param>
+    public void SwapIndexes(int index1, int index2)


Suggested change

public void SwapIndexes(int index1, int index2)

public void SwapIndices(int index1, int index2)

I think the plural goes like that for index.

Yeah, looking it up, indices seems to be the preference outside of North America.

hhyyrylainen · 2023-10-09T06:48:53Z

src/general/HexLayout.cs

@@ -222,7 +237,7 @@ public List<Hex> GetIslandHexes()
        // These are all of the existing hexes, that if there are no islands will all be visited
        var shouldBeVisited = ComputeHexCache();


Why is compute hex cache not given the cutoff parameter here?

hhyyrylainen · 2023-10-09T06:50:28Z

src/early_multicellular_stage/editor/action_data/CellReproductionOrderActionData.cs

+
+    protected override ActionInterferenceMode GetInterferenceModeWithGuaranteed(CombinableActionData other)
+    {
+        return ActionInterferenceMode.NoInterference;


Should this action combine with later reproduction order actions? Similarly to how multiple organelle moves in a row will combine. For example if someone moves the last cell to be the second cell and then moves the second cell to first, that could combine into one reproduction order action, right?

hhyyrylainen · 2023-10-09T06:51:15Z

src/early_multicellular_stage/editor/CellBodyPlanEditorComponent.Callbacks.cs

@@ -8,12 +8,16 @@ public partial class CellBodyPlanEditorComponent
    private void OnCellAdded(HexWithData<CellTemplate> hexWithData)
    {
        cellDataDirty = true;
+
+        UpdateReproductionOrderList();


This should get triggered like the other updates with the cell data dirty. I think there's already a ton of systems that get updated based on the cellDataDirty variable.

hhyyrylainen · 2023-10-09T06:54:19Z

src/late_multicellular_stage/editor/MetaballEditorComponentBase.cs

@@ -565,7 +565,7 @@ protected void OnSymmetryPressed()
        */
    }

-    /// <inheritdoc cref="HexEditorComponentBase{TEditor,TCombinedAction,TAction,THexMove}.OnHexEditorMouseEntered"/>
+    /// <inheritdoc cref="HexEditorComponentBase{TEditor,TCombined,TAction,THexMove,TContext}.OnHexEditorMouseEntered"/>


I think I fixed this on my branch where I fix various other things. I think I fixed this at least a bit better as I kept the right TCombinedAction name here. So maybe you can just revert this change and leave it be for a bit (I'm hopefult it won't take more than two weeks to get my changes regarding this merged)?

src/early_multicellular_stage/editor/ReproductionOrder.cs

…e it generic so it can be reused for organelles. Also, renamed ReproductionOrder to ReproductionOrderEntry

…r GUI control types.

…fuclty of combining those actions

…scene instead of its own version of it.

Kashnox · 2023-10-21T17:58:13Z

This is ready for review again. After splitting the reproduction order handling into its own class, I ended up reworking it to be generic so it should be easy to reuse it for organelle division order too.

I still haven't implemented any combining for ReproductionOrderActions because the more I thought about it, the more complicated it seemed. I think, if we do end up adding an MP cost to reproduction order changes, we should worry about combining the actions then because we'll have a better idea of how exactly they should be combined. In the meantime, I just left a TODO comment in ReproductionOrderActionData related to it.

The Jetbrains Inspectcode is failing because of that bad reference to HexEditorComponentBase, but everything else is passing.

hhyyrylainen · 2023-10-23T06:52:26Z

src/late_multicellular_stage/editor/MetaballEditorComponentBase.cs

@@ -575,14 +575,14 @@ protected void OnMetaballEditorMouseEntered()
        UpdateMutationPointsBar();
    }

-    /// <inheritdoc cref="HexEditorComponentBase{TEditor,TCombinedAction,TAction,THexMove}.OnHexEditorMouseExited"/>
+    /// <inheritdoc cref="HexEditorComponentBase{TEditor,TCombined,TAction,THexMove,TContext}.OnHexEditorMouseExited"/>


There's still changes in this file, so I think you might as well put the correct stuff here (note that due to the line length, there's a need to disable the too long line check for these few lines. You can check my open refactor PR where I did this. Or you could just revert all changes to this file. I think the CI checks will stop checking this file once you make one extra commit after the one reverting the changes in this file.

hhyyrylainen · 2023-10-23T06:53:31Z

src/general/base_stage/HexEditorComponentBase.cs

@@ -1071,6 +1085,7 @@ protected override void Dispose(bool disposing)
                EditorGridPath.Dispose();
                CameraFollowPath.Dispose();
                IslandErrorPath.Dispose();
+                Camera?.Dispose();


We don't dispose Godot nodes. Only nodepaths are disposed.

hhyyrylainen · 2023-10-23T06:54:15Z

src/general/base_stage/HexEditorComponentBase.cs

+    /// <summary>
+    ///   The <see cref="MicrobeCamera"/> used by this editor.
+    /// </summary>
+    public MicrobeCamera? Camera;


If this must be made public, could you at least make this a property without a public setter? That way this class state cannot be destroyed by external code swapping out the camera object.

hhyyrylainen · 2023-10-23T06:57:52Z

src/early_multicellular_stage/editor/CellBodyPlanEditorComponent.cs

@@ -96,6 +99,9 @@ public partial class CellBodyPlanEditorComponent :
    private PackedScene microbeScene = null!;

    private CellPopupMenu cellPopupMenu = null!;
+
+    private ReproductionOrderEditor<CellTemplate, EarlyMulticellularEditor, CombinedEditorAction, EditorAction,


Shouldn't this use the actual type used by the scene (EarlyMulticellularReproductionOrderEditor)? That'd simplify the long list of generic parameters defined here.

hhyyrylainen · 2023-10-23T06:59:58Z

src/early_multicellular_stage/editor/OrderMarginContainer.tscn

As this has an attached script of type EarlyMulticellularReproductionOrderEditor this file should be named EarlyMulticellularReproductionOrderEditor.tscn otherwise it is really hard to find what scenes are related to what script classes if they aren't named the same.

hhyyrylainen · 2023-10-23T07:01:05Z

src/early_multicellular_stage/editor/ReproductionOrderEntry.cs

+using Godot;
+
+/// <summary>
+///   Handles showing and changing the order in which hexes in a hex-based creature will be created.


I think this summary would be work better on the reproduction order editor class. This class for a single entry should have a different summary about the role this class has in the reproduction order editing process.

hhyyrylainen · 2023-10-23T07:01:35Z

src/early_multicellular_stage/editor/ReproductionOrderEntry.cs

+    /// <summary>
+    ///   When this hex will be created. 1 is the starting hex.
+    /// </summary>
+    public string Index


Why is this a string and not an int?

hhyyrylainen · 2023-10-23T07:03:02Z

src/early_multicellular_stage/editor/ReproductionOrderEditor.cs

+    where THexMove : HexWithData<THex>, IActionHex
+{
+    [Export]
+    public NodePath ReproductionOrderListPath = null!;


First NodePath in a class needs to be nullable to allow detecting when this object is created in the Godot editor and avoiding error conditions happening there. Editor process created class instances never get their Export variables set and don't have _Ready called on them, but do have Dispose called on them.

hhyyrylainen · 2023-10-28T10:34:05Z

This now is blocked by the multicellular editor not being currently able to display anything related to the cells. It might take a week or two before I get around to fixing that, but after that this PR can then be finished.

revolutionary-bot · 2023-12-08T13:21:49Z

We are currently in feature freeze until the next release.
If your PR is not just a simple fix, then it may take until the release to get reviewed and merged.

hhyyrylainen · 2024-01-12T09:45:07Z

There's no longer any blockers regarding this, just needs to be finished and updated to work with the latest other code changes.

hhyyrylainen · 2024-11-19T14:23:21Z

I separately implemented a GUI for growth order changing for organelles: #5688

It has all the features like drag and drop and I think if this work is resumed it might be a good idea to do this again based on that code.

Kashnox added 3 commits September 23, 2023 16:47

Added the framework for changing and seeing the reproduction order of…

a429b0c

… early multicellular species.

Started checking for islands in the reproduction order, coloring thos…

ce1463b

…e red, and blocking the user from closing the editor while they exist. Also started showing the reproduction order on the cells while the Reproduction tab is open.

Merge branch 'master' into 3206_cell_division_order

1044b1d

hhyyrylainen added this to the Release 0.6.4 milestone Oct 1, 2023

Kashnox added 10 commits October 1, 2023 19:04

Added support for undoing and redoing reproduction order changes

5b7d4df

Added a heading for the growth order and gave it a tooltip

eadbe93

Removing placeholder text from localization

9794e24

Added BOM

5b74174

Added new line to csproj file

362e1ff

Fixing localization

eb956d5

Code cleanup

44fcb42

Added PLACEHOLDER to placeholder text

0850e2f

Fixed missing xml references

f2f104f

Removed unndeeded Font reference

1b61841

Kashnox marked this pull request as ready for review October 7, 2023 18:31

Kashnox requested review from a team October 7, 2023 18:31

Kashnox added the review label Oct 7, 2023

hhyyrylainen reviewed Oct 9, 2023

View reviewed changes

src/early_multicellular_stage/editor/ReproductionOrder.cs Outdated Show resolved Hide resolved

hhyyrylainen reviewed Oct 9, 2023

View reviewed changes

src/early_multicellular_stage/editor/ReproductionOrder.cs Outdated Show resolved Hide resolved

Kashnox added 4 commits October 21, 2023 11:43

Moved reproduction order handling to its own script and scene and mad…

0b76d9b

…e it generic so it can be reused for organelles. Also, renamed ReproductionOrder to ReproductionOrderEntry

Updated the way things are disposed for the new classes to match othe…

0c7b783

…r GUI control types.

Left a todo comment in ReproductionOrderActionData describing the dif…

70a3bd1

…fuclty of combining those actions

Updated localization

9545f7f

Kashnox added 3 commits October 21, 2023 13:35

Added new line back to the end of Thrive.csproj

d363ab6

Added BOM

2077618

Switched CellBodyPlanEditorComponent to use the OrderMarginContainer …

3c040c0

…scene instead of its own version of it.

Kashnox requested a review from a team October 21, 2023 17:58

hhyyrylainen reviewed Oct 23, 2023

View reviewed changes

hhyyrylainen modified the milestones: Release 0.6.4, Release 0.6.5 Dec 8, 2023

hhyyrylainen modified the milestones: Release 0.6.5, Release 0.6.6 Feb 16, 2024

hhyyrylainen modified the milestones: Release 0.6.6, Release 0.6.7 Apr 25, 2024

hhyyrylainen modified the milestones: Release 0.6.7, Release 0.6.8 Jun 7, 2024

hhyyrylainen removed this from the Release 0.7.0 milestone Aug 30, 2024

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Implement a way to edit the cell division order in multicellular #4499

Implement a way to edit the cell division order in multicellular #4499

Kashnox commented Oct 1, 2023 •

edited

Loading

hhyyrylainen Oct 9, 2023

Kashnox Oct 10, 2023

hhyyrylainen Oct 9, 2023

hhyyrylainen Oct 9, 2023

hhyyrylainen Oct 9, 2023

hhyyrylainen Oct 9, 2023

Kashnox commented Oct 21, 2023

hhyyrylainen Oct 23, 2023

hhyyrylainen Oct 23, 2023

hhyyrylainen Oct 23, 2023

hhyyrylainen Oct 23, 2023

hhyyrylainen Oct 23, 2023

hhyyrylainen Oct 23, 2023

hhyyrylainen Oct 23, 2023

hhyyrylainen Oct 23, 2023

hhyyrylainen commented Oct 28, 2023

revolutionary-bot commented Dec 8, 2023

hhyyrylainen commented Jan 12, 2024

hhyyrylainen commented Nov 19, 2024

	public void SwapIndexes(int index1, int index2)
	public void SwapIndices(int index1, int index2)

		@@ -222,7 +237,7 @@ public List<Hex> GetIslandHexes()
		// These are all of the existing hexes, that if there are no islands will all be visited
		var shouldBeVisited = ComputeHexCache();

Implement a way to edit the cell division order in multicellular #4499

Are you sure you want to change the base?

Implement a way to edit the cell division order in multicellular #4499

Conversation

Kashnox commented Oct 1, 2023 • edited Loading

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

Kashnox commented Oct 21, 2023

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

hhyyrylainen commented Oct 28, 2023

revolutionary-bot commented Dec 8, 2023

hhyyrylainen commented Jan 12, 2024

hhyyrylainen commented Nov 19, 2024

Kashnox commented Oct 1, 2023 •

edited

Loading