feature/showcase_bugfixing_kathi_partII #16

Merged
Jonathan merged 33 commits from feature/showcase_bugfixing_kathi_partII into develop 2025-10-24 18:31:56 +02:00
5 changed files with 13 additions and 10 deletions
Showing only changes of commit 871e1856f1 - Show all commits
-1
View File
@@ -2170,7 +2170,6 @@ script = ExtResource("475_nxglm")
[connection signal="FilledWateringCan" from="." to="CharacterBody2D/WateringCanUI" method="Refill"]
[connection signal="InventorySelectionChanged" from="." to="CharacterBody2D/WateringCanUI" method="IsWateringCanActive"]
[connection signal="PickedUpTool" from="." to="CharacterBody2D" method="ActivateTool"]
[connection signal="PickedUpTool" from="." to="CharacterBody2D/visuals" method="ActivateTool"]
[connection signal="PickedUpTool" from="." to="CharacterBody2D/WateringCanUI" method="IsWateringCanActive"]
[connection signal="WateringField" from="FarmingControls" to="CharacterBody2D/visuals" method="PlayWateringAnimation"]
+6 -1
View File
@@ -1160,6 +1160,7 @@ region_rect = Rect2(130, 0, 201, 278)
[node name="bush6" type="Sprite2D" parent="YSorted/Farm visuals/Static/greenery/left side"]
modulate = Color(0.8428, 0.8771, 0.98, 1)
z_index = 2
material = SubResource("ShaderMaterial_bcdgk")
position = Vector2(2612, 4022)
scale = Vector2(2, 2)
@@ -1255,6 +1256,7 @@ region_enabled = true
region_rect = Rect2(1699, 76, 280, 230)
[node name="bush14" type="Sprite2D" parent="YSorted/Farm visuals/Static/greenery/left side"]
z_index = 2
material = SubResource("ShaderMaterial_bcdgk")
position = Vector2(8925, 4194)
scale = Vector2(-5.54387, 3.80466)
@@ -1515,6 +1517,7 @@ region_enabled = true
region_rect = Rect2(1699, 76, 280, 230)
[node name="bush13" type="Sprite2D" parent="YSorted/Farm visuals/Static/greenery/right side"]
z_index = 2
material = SubResource("ShaderMaterial_bcdgk")
position = Vector2(8145, 4141)
scale = Vector2(-2.82886, 2.51195)
@@ -1524,6 +1527,7 @@ region_enabled = true
region_rect = Rect2(1699, 76, 280, 230)
[node name="bush14" type="Sprite2D" parent="YSorted/Farm visuals/Static/greenery/right side"]
z_index = 2
material = SubResource("ShaderMaterial_bcdgk")
position = Vector2(8925, 4194)
scale = Vector2(-5.54387, 3.80466)
@@ -1534,6 +1538,7 @@ region_rect = Rect2(130, 0, 201, 278)
[node name="bush15" type="Sprite2D" parent="YSorted/Farm visuals/Static/greenery/right side"]
modulate = Color(0.8428, 0.8771, 0.98, 1)
z_index = 2
material = SubResource("ShaderMaterial_bcdgk")
position = Vector2(7763, 4164)
scale = Vector2(3.21806, 3.18583)
@@ -1800,7 +1805,7 @@ region_enabled = true
region_rect = Rect2(29, 204, 219, 159)
[node name="grass27" type="Sprite2D" parent="YSorted/Farm visuals/Static/greenery/grass"]
z_index = -5
z_index = 2
position = Vector2(7757, 3423)
rotation = 1.57079
scale = Vector2(7.28513, 6.16997)
@@ -1,4 +1,3 @@
using System.Threading.Tasks;
using Babushka.scripts.CSharp.Common.Inventory;
using Babushka.scripts.CSharp.Common.Services;
using Godot;
@@ -46,23 +46,21 @@ public partial class VesnaBehaviour2D : Node
{
InventorySlot currentSlot = InventoryManager.Instance.GetCurrentSelectedSlot();
ItemInstance? currentItem = currentSlot.itemInstance;
if (currentItem == null)
return;
int toolId = -1;
if (currentItem.blueprint == _hoe)
if (currentItem != null && currentItem.blueprint == _hoe)
{
toolId = 0;
}
if (currentItem.blueprint == _wateringCan)
if (currentItem != null && currentItem.blueprint == _wateringCan)
{
toolId = 1;
}
ActivateTool(toolId);
_vesnaAnimations.ActivateTool(toolId >= 0, toolId);
EmitSignal(SignalName.InventorySelectionChanged, toolId);
}
@@ -1,4 +1,5 @@
using Godot;
namespace Babushka.scripts.CSharp.Common.Inventory;
[GlobalClass]
@@ -18,7 +19,7 @@ public partial class ItemResource : Resource
[Export]
Outdated
Review

Ich musste im Code nachschauen, um zu verstehen, wofür das überhaupt ist. Ich denke, das braucht mindestens einen anderen Namen.
Besser aber noch, das kommt in eine neue Klasse oder eine abgeleitete Klasse (z. B. PlantableItemResource)

Außerdem habe ich Angst, da hier ne PackedScene referenziert wird, dass wir wieder irgendwann in ein cyclic dependency problem geraten, das super schwer zu debuggen ist.

Ich musste im Code nachschauen, um zu verstehen, wofür das überhaupt ist. Ich denke, das braucht mindestens einen anderen Namen. Besser aber noch, das kommt in eine neue Klasse oder eine abgeleitete Klasse (z. B. `PlantableItemResource`) Außerdem habe ich Angst, da hier ne PackedScene referenziert wird, dass wir wieder irgendwann in ein cyclic dependency problem geraten, das super schwer zu debuggen ist.
Outdated
Review

Ich habe noch einmal darüber nachgedacht und bin mir sehr sicher, dass wir damit in eine cyclic dependency geraten, sobald eine Tomatenpflanze nicht nur Tomaten, sondern auch selbst wieder Samen droped. Selbst wenn das Gamedesign technisch nicht vorgesehen ist, schaffen wir uns hier einen ganz gefährlichen Sonderfall.

Wir sollten stattdessen lieber eine Lookup-Table-Resource haben. Darin ist ein Array aus Paaren von ItemResource und PackedScene. Die FieldBehaviour Klasse kann sich basierend auf dem Item das richtige Prefab für die Pflanze raus suchen

Ich habe noch einmal darüber nachgedacht und bin mir sehr sicher, dass wir damit in eine cyclic dependency geraten, sobald eine Tomatenpflanze nicht nur Tomaten, sondern auch selbst wieder Samen droped. Selbst wenn das Gamedesign technisch nicht vorgesehen ist, schaffen wir uns hier einen ganz gefährlichen Sonderfall. Wir sollten stattdessen lieber eine Lookup-Table-Resource haben. Darin ist ein Array aus Paaren von ItemResource und PackedScene. Die FieldBehaviour Klasse kann sich basierend auf dem Item das richtige Prefab für die Pflanze raus suchen
Outdated
Review

Lass mal bei Gelegenheit darüber callen.

Ich bin mir fast sicher, dass deine Bedenken hier unbegründet sind. Der Lösungsvorschlag mit der Lookup-Tabelle ließe sich zwar auch umsetzen, allerdings nicht mit ItemResources als Keys, weil diese nicht von Godot serialisiert werden können und damit in Godot Dictionaries Fehler werfen.

Man kann das umgehen, indem man strings oder enums als identifier benutzt und dazwischenschaltet, das ist aber im Verhältnis zu dieser Lösung sehr viel komplexer und meiner Meinung nach nicht notwendig, da wir ja nicht auf Instanzen direkt zugreifen, sondern nur auf Prefabs / Resourcen verweisen.

Lass mal bei Gelegenheit darüber callen. Ich bin mir fast sicher, dass deine Bedenken hier unbegründet sind. Der Lösungsvorschlag mit der Lookup-Tabelle ließe sich zwar auch umsetzen, allerdings nicht mit ItemResources als Keys, weil diese nicht von Godot serialisiert werden können und damit in Godot Dictionaries Fehler werfen. Man kann das umgehen, indem man strings oder enums als identifier benutzt und dazwischenschaltet, das ist aber im Verhältnis zu dieser Lösung sehr viel komplexer und meiner Meinung nach nicht notwendig, da wir ja nicht auf Instanzen direkt zugreifen, sondern nur auf Prefabs / Resourcen verweisen.
public PackedScene? itemPrefab;
public ItemResource()
{
name = "";
@@ -27,3 +28,4 @@ public partial class ItemResource : Resource
itemPrefab = null;
}
}