-
-
Notifications
You must be signed in to change notification settings - Fork 521
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Add dots indicating player population on patch map #5880
base: master
Are you sure you want to change the base?
Changes from 17 commits
cb4ceec
608718f
4268a2c
5d8212c
86c2910
1e9ddfa
4efda91
797aea9
e67f36a
03603a3
fa9f846
20de572
d55eb81
63ccc78
194cc1b
98a1bec
c168bc5
d1752e5
a8fd79c
80a7e8c
330293e
fcf8f69
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,37 @@ | ||
[remap] | ||
|
||
importer="texture" | ||
type="CompressedTexture2D" | ||
uid="uid://cbkfqk0lk6dhl" | ||
path="res://.godot/imported/MapDotIndicator.svg-0118f3fb709521b7aea1c36f416e97e1.ctex" | ||
metadata={ | ||
"vram_texture": false | ||
} | ||
|
||
[deps] | ||
|
||
source_file="res://assets/textures/gui/bevel/MapDotIndicator.svg" | ||
dest_files=["res://.godot/imported/MapDotIndicator.svg-0118f3fb709521b7aea1c36f416e97e1.ctex"] | ||
|
||
[params] | ||
|
||
compress/mode=0 | ||
compress/high_quality=false | ||
compress/lossy_quality=0.7 | ||
compress/hdr_compression=1 | ||
compress/normal_map=0 | ||
compress/channel_pack=0 | ||
mipmaps/generate=true | ||
mipmaps/limit=-1 | ||
roughness/mode=0 | ||
roughness/src_normal="" | ||
process/fix_alpha_border=true | ||
process/premult_alpha=false | ||
process/normal_map_invert_y=false | ||
process/hdr_as_srgb=false | ||
process/hdr_clamp_exposure=false | ||
process/size_limit=0 | ||
detect_3d/compress_to=1 | ||
svg/scale=1.0 | ||
editor/scale_with_editor_scale=false | ||
editor/convert_colors_with_editor_theme=false |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -1705,6 +1705,9 @@ public static class Constants | |
|
||
public const float PATCH_GENERATION_CHANCE_BANANA_BIOME = 0.03f; | ||
|
||
// Constants for displaying the patch map | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This comment doesn't seem to match what this is on? This should be an xml summary comment along the lines of "Sets how many population indicator dots appear on the patch map" |
||
public const float PLAYER_POPULATION_POPULATION_FOR_PER_INDICATOR = 1.0f / 300.0f; | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Shouldn't the name of this constant be the inverse? Because this is how many indicators there are per population. That's the difference between |
||
|
||
/// <summary> | ||
/// If set to true, then physics debug draw gets enabled when the game starts | ||
/// </summary> | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -26,6 +26,8 @@ public partial class PatchMapDrawer : Control | |
[Export] | ||
public NodePath LineContainerPath = null!; | ||
|
||
public uint PlayerSpeciesID = int.MaxValue; | ||
|
||
#pragma warning disable CA2213 | ||
[Export] | ||
public ShaderMaterial MonochromeMaterial = null!; | ||
|
@@ -47,6 +49,9 @@ public partial class PatchMapDrawer : Control | |
private PackedScene nodeScene = null!; | ||
private Control patchNodeContainer = null!; | ||
private Control lineContainer = null!; | ||
|
||
[Export] | ||
private Control populationIndicatorContainer = null!; | ||
#pragma warning restore CA2213 | ||
|
||
private PatchMap map = null!; | ||
|
@@ -55,6 +60,8 @@ public partial class PatchMapDrawer : Control | |
|
||
private bool alreadyDrawn; | ||
|
||
private List<Control> playerSpeciesPopulationIndicators = new(); | ||
|
||
private Dictionary<Patch, bool>? patchEnableStatusesToBeApplied; | ||
|
||
private Patch? selectedPatch; | ||
|
@@ -1109,10 +1116,67 @@ private void AddPatchNode(Patch patch, Vector2 position) | |
|
||
patch.ApplyPatchEventVisuals(node); | ||
|
||
var playerSpecies = patch.FindSpeciesByID(PlayerSpeciesID); | ||
if (playerSpecies != null) | ||
{ | ||
AddPlayerPopulationIndicators(patch, playerSpecies, node, position); | ||
} | ||
|
||
patchNodeContainer.AddChild(node); | ||
nodes.Add(node.Patch, node); | ||
} | ||
|
||
private void AddPlayerPopulationIndicators(Patch patch, Species playerSpecies, Control node, Vector2 position) | ||
{ | ||
var playerPopulationIndicatorAmount = (int)Math.Ceiling(patch.GetSpeciesSimulationPopulation(playerSpecies) * | ||
Constants.PLAYER_POPULATION_POPULATION_FOR_PER_INDICATOR); | ||
|
||
var indicatorExcess = Math.Clamp(playerSpeciesPopulationIndicators.Count - playerPopulationIndicatorAmount, | ||
0, | ||
playerSpeciesPopulationIndicators.Count); | ||
|
||
// Hide excess from the end of the list | ||
for (int i = 0; i < indicatorExcess; ++i) | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. If you want to keep things this way, I'll accept it, but I think a slightly easier way to program this would be to use a variable like, There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Now with the persistent index for used indicators, I think this hiding excess indicators code needs to be put into a single place that is called after the entire map is drawn. Otherwise the last indicators get hidden over and over (and I think your indicatorExcess math was not updated anyway so that would lead to a bug in any case). |
||
{ | ||
playerSpeciesPopulationIndicators[playerSpeciesPopulationIndicators.Count - i].Hide(); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Also when I'm looking at this code, isn't the math wrong? Because Also I think I just realized also another problem: the player being in multiple patches will reset the indicators. There needs to be a persistent |
||
} | ||
|
||
for (int i = 0; i < playerPopulationIndicatorAmount; ++i) | ||
{ | ||
var noCached = i >= playerSpeciesPopulationIndicators.Count; | ||
|
||
Control indicator; | ||
if (noCached) | ||
{ | ||
indicator = new TextureRect | ||
{ | ||
Texture = GD.Load<Texture2D>("res://assets/textures/gui/bevel/MapDotIndicator.svg"), | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This texture should be loaded just once in _Ready and not tried to be reloaded for each indicator. |
||
ExpandMode = TextureRect.ExpandModeEnum.IgnoreSize, | ||
Size = new Vector2(6, 6), | ||
}; | ||
|
||
populationIndicatorContainer.AddChild(indicator); | ||
} | ||
else | ||
{ | ||
indicator = playerSpeciesPopulationIndicators[i]; | ||
|
||
indicator.Show(); | ||
} | ||
|
||
indicator.MouseFilter = MouseFilterEnum.Ignore; | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This should be in the indicator creation as it doesn't seem like this value is ever updated so it wastes time to reapply the value on each loop iteration. |
||
|
||
var nodeModifier = node.Position.LengthSquared(); | ||
var modifierSinus = MathF.Sin(i); | ||
|
||
playerSpeciesPopulationIndicators.Add(indicator); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Isn't this a major bug where the same indicator objects are added again and again to the same list? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I think this is probably what caused this error when I moved patches in the editor:
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Good you caught that. |
||
|
||
indicator.Position = position + node.Size * 0.5f + new Vector2(0, 20) | ||
.Rotated(nodeModifier * 30) + | ||
new Vector2(0, modifierSinus * 50).Rotated(i * 6 * modifierSinus + nodeModifier); | ||
} | ||
} | ||
|
||
private void BuildPatchToRegionConnections(PatchRegion region1, PatchRegion region2, Vector2 regionPoint, | ||
Vector2 regionPreviousPoint, MapElementVisibility regionVisibility) | ||
{ | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,23 +1,34 @@ | ||
[gd_scene load_steps=4 format=2] | ||
[gd_scene load_steps=4 format=3 uid="uid://ui60hm6q35fb"] | ||
|
||
[ext_resource path="res://src/microbe_stage/editor/PatchMapDrawer.cs" type="Script" id=1] | ||
[ext_resource path="res://shaders/Monochrome.gdshader" type="Shader" id=2] | ||
[ext_resource type="Script" path="res://src/microbe_stage/editor/PatchMapDrawer.cs" id="1"] | ||
[ext_resource type="Shader" path="res://shaders/Monochrome.gdshader" id="2"] | ||
|
||
[sub_resource type="ShaderMaterial" id=1] | ||
shader = ExtResource( 2 ) | ||
[sub_resource type="ShaderMaterial" id="1"] | ||
shader = ExtResource("2") | ||
|
||
[node name="PatchMapDrawer" type="Control"] | ||
[node name="PatchMapDrawer" type="Control" node_paths=PackedStringArray("populationIndicatorContainer")] | ||
layout_mode = 3 | ||
anchors_preset = 15 | ||
anchor_right = 1.0 | ||
anchor_bottom = 1.0 | ||
mouse_filter = 1 | ||
grow_horizontal = 2 | ||
grow_vertical = 2 | ||
size_flags_horizontal = 3 | ||
size_flags_vertical = 3 | ||
script = ExtResource( 1 ) | ||
mouse_filter = 1 | ||
script = ExtResource("1") | ||
DrawDefaultMapIfEmpty = true | ||
PatchNodeContainerPath = NodePath("PatchNodeContainer") | ||
LineContainerPath = NodePath("LineContainer") | ||
MonochromeMaterial = SubResource( 1 ) | ||
MonochromeMaterial = SubResource("1") | ||
populationIndicatorContainer = NodePath("PopulationIndicatorContainer") | ||
|
||
[node name="PopulationIndicatorContainer" type="Control" parent="."] | ||
layout_mode = 3 | ||
anchors_preset = 0 | ||
|
||
[node name="LineContainer" type="Control" parent="."] | ||
anchors_preset = 0 | ||
|
||
[node name="PatchNodeContainer" type="Control" parent="."] | ||
anchors_preset = 0 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think that maybe if the dot was fully filled in and made slightly less in opacity, it would be a lot more subtle effect? That way I think there wouldn't be really any complaining about the added visuals.