From 448fd660c52d0e4ab913433443e026fa05e6f317 Mon Sep 17 00:00:00 2001 From: Joanna May Date: Tue, 7 Nov 2023 23:28:10 -0600 Subject: [PATCH] feat: be compatible with GodotNodeInterfaces --- .../Chickensoft.PowerUps.Tests.csproj | 2 +- .../badges/branch_coverage.svg | 2 +- .../badges/line_coverage.svg | 2 +- Chickensoft.PowerUps.Tests/src/AutoDispose.cs | 114 --------- Chickensoft.PowerUps.Tests/src/AutoNode.cs | 242 +++--------------- .../src/FakeNodeTree.cs | 128 --------- Chickensoft.PowerUps.Tests/src/IAutoNode.cs | 217 ---------------- .../test/fixtures/AutoDisposeTestScene.cs | 26 -- .../test/fixtures/AutoDisposeTestScene.tscn | 6 - .../test/fixtures/DisposableObject.cs | 12 - .../test/src/AutoDisposeTest.cs | 72 ------ .../test/src/AutoNodeInvalidCastTest.cs | 13 + .../Chickensoft.PowerUps.csproj | 2 +- README.md | 78 ++---- global.json | 4 +- 15 files changed, 84 insertions(+), 836 deletions(-) delete mode 100644 Chickensoft.PowerUps.Tests/src/AutoDispose.cs delete mode 100644 Chickensoft.PowerUps.Tests/src/FakeNodeTree.cs delete mode 100644 Chickensoft.PowerUps.Tests/src/IAutoNode.cs delete mode 100644 Chickensoft.PowerUps.Tests/test/fixtures/AutoDisposeTestScene.cs delete mode 100644 Chickensoft.PowerUps.Tests/test/fixtures/AutoDisposeTestScene.tscn delete mode 100644 Chickensoft.PowerUps.Tests/test/fixtures/DisposableObject.cs delete mode 100644 Chickensoft.PowerUps.Tests/test/src/AutoDisposeTest.cs diff --git a/Chickensoft.PowerUps.Tests/Chickensoft.PowerUps.Tests.csproj b/Chickensoft.PowerUps.Tests/Chickensoft.PowerUps.Tests.csproj index 8c499a0..4cd23f9 100644 --- a/Chickensoft.PowerUps.Tests/Chickensoft.PowerUps.Tests.csproj +++ b/Chickensoft.PowerUps.Tests/Chickensoft.PowerUps.Tests.csproj @@ -21,6 +21,6 @@ - + diff --git a/Chickensoft.PowerUps.Tests/badges/branch_coverage.svg b/Chickensoft.PowerUps.Tests/badges/branch_coverage.svg index f49b59a..21cd0e7 100644 --- a/Chickensoft.PowerUps.Tests/badges/branch_coverage.svg +++ b/Chickensoft.PowerUps.Tests/badges/branch_coverage.svg @@ -94,7 +94,7 @@ - Generated by: ReportGenerator 5.1.17.0 + Generated by: ReportGenerator 5.1.26.0 diff --git a/Chickensoft.PowerUps.Tests/badges/line_coverage.svg b/Chickensoft.PowerUps.Tests/badges/line_coverage.svg index fcdc4e5..e1f11a9 100644 --- a/Chickensoft.PowerUps.Tests/badges/line_coverage.svg +++ b/Chickensoft.PowerUps.Tests/badges/line_coverage.svg @@ -94,7 +94,7 @@ - Generated by: ReportGenerator 5.1.17.0 + Generated by: ReportGenerator 5.1.26.0 diff --git a/Chickensoft.PowerUps.Tests/src/AutoDispose.cs b/Chickensoft.PowerUps.Tests/src/AutoDispose.cs deleted file mode 100644 index 3b0058a..0000000 --- a/Chickensoft.PowerUps.Tests/src/AutoDispose.cs +++ /dev/null @@ -1,114 +0,0 @@ -namespace Chickensoft.PowerUps; - -using System; -using System.Collections.Generic; -using System.Collections.Immutable; -using Godot; -using SuperNodes.Types; - -#pragma warning disable CS8019 -using Chickensoft.PowerUps; -#pragma warning restore CS8019 - -/// -/// Interface for a PowerUp that automatically disposes of any writeable -/// properties or fields on your node that are references to IDisposable (but -/// not nodes โ€” Godot cleans those up for you). -/// -public interface IAutoDispose { } - -/// -/// Add this PowerUp to a SuperNode to automatically dispose of any writeable -/// properties or fields on your node that are references to IDisposable (but -/// not nodes โ€” Godot cleans those up for you). -/// -[PowerUp] -public abstract partial class AutoDispose : Node, IAutoDispose { - #region ISuperNode - // These don't need to be copied over since we will be copied into an - // ISuperNode. - - [PowerUpIgnore] - public ImmutableDictionary PropertiesAndFields - => throw new NotImplementedException(); - [PowerUpIgnore] - public TResult GetScriptPropertyOrFieldType( - string scriptProperty, ITypeReceiver receiver - ) => throw new NotImplementedException(); - [PowerUpIgnore] - public dynamic GetScriptPropertyOrField(string scriptProperty) => - throw new NotImplementedException(); - - #endregion ISuperNode - - #region StatefulMixinAdditions - public HashSet Disposables { get; } = new(); - #endregion StatefulMixinAdditions - - public void OnAutoDispose(int what) { - if (what == NotificationReady) { - // After the node is ready, register all the disposables it has setup. - ToSignal(this, Node.SignalName.Ready).OnCompleted( - () => Disposer.RegisterDisposables( - PropertiesAndFields, Disposables, (ISuperNode)this - ) - ); - } - else if (what == NotificationExitTree) { - // After the node has exited, cleanup the disposables it registered. - ToSignal(this, Node.SignalName.TreeExited).OnCompleted( - () => Disposer.DisposeDisposables(Disposables) - ); - } - } -} - -public static class Disposer { - public class IsDisposableChecker : ITypeReceiver { - /// Object to check to see if it is disposable. - public object? PotentialDisposable { get; set; } - - public IsDisposableChecker() { - PotentialDisposable = default; - } - -#nullable disable - public IDisposable Receive() => - PotentialDisposable is IDisposable disposable and not Node - ? disposable - : default; -#nullable restore - } - - [ThreadStatic] - private static readonly IsDisposableChecker _checker = new(); - - public static void RegisterDisposables( - ImmutableDictionary propertiesAndFields, - HashSet disposables, - ISuperNode node - ) { - foreach (var (name, propertyOrField) in propertiesAndFields) { - if (!propertyOrField.IsReadable || !propertyOrField.IsMutable) { - continue; - } - // Only auto-dispose readable and writeable properties - _checker.PotentialDisposable = node.GetScriptPropertyOrField(name); - if ( - node.GetScriptPropertyOrFieldType(name, _checker) is - IDisposable disposable - ) { - disposables.Add(disposable); - } - } - } - - public static void DisposeDisposables(HashSet disposables) { - lock (disposables) { - foreach (var obj in disposables) { - if (obj is IDisposable disposable) { disposable.Dispose(); } - } - disposables.Clear(); - } - } -} diff --git a/Chickensoft.PowerUps.Tests/src/AutoNode.cs b/Chickensoft.PowerUps.Tests/src/AutoNode.cs index a04d426..56aefad 100644 --- a/Chickensoft.PowerUps.Tests/src/AutoNode.cs +++ b/Chickensoft.PowerUps.Tests/src/AutoNode.cs @@ -37,6 +37,12 @@ public NodeAttribute(string? path = null) { public const string ID = "global::Chickensoft.PowerUps.NodeAttribute"; } +/// +/// Apply this PowerUp to your SuperNode to automatically connect declared node +/// references to their corresponding instances in the scene tree. +/// +public interface IAutoNode : IFakeNodeTreeEnabled, ISuperNode { } + /// /// Apply this PowerUp to your SuperNode to automatically connect declared node /// references to their corresponding instances in the scene tree. @@ -53,188 +59,6 @@ public void FakeNodeTree( System.Collections.Generic.Dictionary? nodes ) => FakeNodes = new FakeNodeTree(this, nodes); - #region IAutoNode - public void AddChildEx( - object node, - bool forceReadableName = false, - InternalMode @internal = - InternalMode.Disabled - ) { - if (node is INodeAdapter adapter) { - // If it's an adapter, we can add the underlying node directly. - AddChild(adapter.Object, forceReadableName, @internal); - return; - } - - if (node is Node godotNode) { - // If it's a Godot node, we can add it directly. - AddChild(godotNode, forceReadableName, @internal); - return; - } - - if (node is INode iNode) { - // We can only add nodes by interface only when we are in a test - // environment, so check to see if that's been setup. - if (FakeNodes is not FakeNodeTree fakeNodeTree) { - throw new InvalidOperationException( - "Fake node tree has not been initialized. If you are attempting to " + - "unit test a node scene, make sure that you have called " + - "node.FakeNodeTree() to initialize the fake node tree." - ); - } - - // We are running in a test environment. - fakeNodeTree.AddChild(iNode); - return; - } - - throw new InvalidOperationException( - $"Cannot add child of type {node.GetType()} to {GetType()}." - ); - } - - public INode? FindChildEx(string pattern, bool recursive, bool owned) { - if (FakeNodes is FakeNodeTree fakeNodeTree) { - return fakeNodeTree.FindChild(pattern); - } - var node = FindChild(pattern, recursive, owned); - return node is null ? null : GodotInterfaces.AdaptNode(node); - } - - public INode[] FindChildrenEx( - string pattern, string type = "", bool recursive = true, bool owned = true - ) { - if (FakeNodes is FakeNodeTree fakeNodeTree) { - return fakeNodeTree.FindChildren(pattern); - } - var nodes = FindChildren(pattern, type, recursive, owned); - var adaptedNodes = new System.Collections.Generic.List(nodes.Count); - foreach (var node in nodes) { - adaptedNodes.Add(GodotInterfaces.AdaptNode(node)); - } - return adaptedNodes.ToArray(); - } - - public T GetChildEx(int idx, bool includeInternal = false) - where T : class, INode { - if (FakeNodes is FakeNodeTree fakeNodeTree) { - return fakeNodeTree.GetChild(idx)!; - } - var node = GetChild(idx, includeInternal); - return GodotInterfaces.Adapt(node); - } - - public INode GetChildEx(int idx, bool includeInternal) { - if (FakeNodes is FakeNodeTree fakeNodeTree) { - return fakeNodeTree.GetChild(idx); - } - var node = GetChild(idx, includeInternal); - return GodotInterfaces.AdaptNode(node); - } - - public T? GetChildOrNullEx( - int idx, bool includeInternal = false - ) where T : class, INode { - // TODO: This uses GetNode under-the-hood since there's no non-generic - // overload for GetChildOrNull. GetChild will cause a warning from Godot if - // the node is actually null, and we can't pass through the generics since - // they are expected to be the node's interface. - // - // The fix for this is to add a method to GodotInterfaces that will invoke - // a callback with the generic type of the corresponding Godot node for any - // given interface type. - if (FakeNodes is FakeNodeTree fakeNodeTree) { - return fakeNodeTree.GetChild(idx); - } - var node = GetChild(idx, includeInternal); - return GodotInterfaces.Adapt(node); - } - - public int GetChildCountEx(bool includeInternal = false) => - FakeNodes is FakeNodeTree fakeNodeTree - ? fakeNodeTree.GetChildCount() - : GetChildCount(includeInternal); - - public INode[] GetChildrenEx(bool includeInternal = false) { - if (FakeNodes is FakeNodeTree fakeNodeTree) { - return fakeNodeTree.GetChildren(); - } - var nodes = GetChildren(includeInternal); - var adaptedNodes = new System.Collections.Generic.List(nodes.Count); - foreach (var node in nodes) { - adaptedNodes.Add(GodotInterfaces.AdaptNode(node)); - } - return adaptedNodes.ToArray(); - } - - public INode GetNodeEx(NodePath path) { - if (FakeNodes is FakeNodeTree fakeNodeTree) { - return fakeNodeTree.GetNode(path)!; - } - var node = GetNode(path); - return node is null ? default! : GodotInterfaces.AdaptNode(node); - } - - public INode? GetNodeOrNullEx(NodePath path) { - if (FakeNodes is FakeNodeTree fakeNodeTree) { - return fakeNodeTree.GetNode(path); - } - var node = GetNodeOrNull(path); - return node is null - ? null - : (INode)GodotInterfaces.AdaptNode(node); - } - - public T? GetNodeOrNullEx(NodePath path) where T : class, INode { - if (FakeNodes is FakeNodeTree fakeNodeTree) { - return fakeNodeTree.GetNode(path); - } - var node = GetNodeOrNull(path); - return node is null - ? null - : GodotInterfaces.Adapt(node); - } - - public bool HasNodeEx(NodePath path) => - FakeNodes is FakeNodeTree fakeNodeTree - ? fakeNodeTree.HasNode(path) - : HasNode(path); - - public void RemoveChildEx(object node) { - if (node is INodeAdapter adapter) { - // If it's an adapter, we can remove the underlying node directly. - RemoveChild(adapter.Object); - return; - } - - if (node is Node godotNode) { - // If it's a Godot node, we can remove it directly. - RemoveChild(godotNode); - return; - } - - if (node is INode iNode) { - // We can remove nodes by interface only when we are in a test - // environment, so check to see if that's been setup. - if (FakeNodes is not FakeNodeTree fakeNodeTree) { - throw new InvalidOperationException( - "Fake node tree has not been initialized. If you are attempting to " + - "unit test a node scene, make sure that you have called " + - "node.FakeNodeTree() to initialize the fake node tree." - ); - } - - // We are running in a test environment. - fakeNodeTree.RemoveChild(iNode); - return; - } - - throw new InvalidOperationException( - $"Cannot remove child of type {node.GetType()} from {GetType()}." - ); - } - #endregion IAutoNode - #region ISuperNode // These don't need to be copied over since we will be copied into an // ISuperNode. @@ -242,13 +66,16 @@ public void RemoveChildEx(object node) { [PowerUpIgnore] public ImmutableDictionary PropertiesAndFields => throw new NotImplementedException(); + [PowerUpIgnore] public TResult GetScriptPropertyOrFieldType( string scriptProperty, ITypeReceiver receiver ) => throw new NotImplementedException(); + [PowerUpIgnore] public dynamic GetScriptPropertyOrField(string scriptProperty) => throw new NotImplementedException(); + [PowerUpIgnore] public void SetScriptPropertyOrField(string scriptProperty, dynamic? value) => throw new NotImplementedException(); @@ -289,6 +116,8 @@ IAutoNode autoNode var path = nodeAttribute.ArgumentExpressions[0] as string ?? AsciiToPascalCase(name); + Exception? e; + // First, check to see if the node has been faked for testing. // Faked nodes take precedence over real nodes. if (autoNode.FakeNodes?.GetNode(path) is INode fakeNode) { @@ -298,12 +127,14 @@ IAutoNode autoNode var satisfiesFakeType = autoNode.GetScriptPropertyOrFieldType(name, _checker); if (!satisfiesFakeType) { - throw new InvalidOperationException( + e = new InvalidOperationException( $"Found a faked node at '{path}' of type " + $"'{fakeNode.GetType().Name}' that is not the expected type " + $"'{propertyOrField.Type.Name}' for member '{name}' on " + $"'{node.Name}'." ); + GD.PushError(e.Message); + throw e; } // Faked node satisfies the expected type :) autoNode.SetScriptPropertyOrField(name, fakeNode); @@ -311,23 +142,27 @@ IAutoNode autoNode } // We're dealing with what should be an actual node in the tree. - var godotNode = node.GetNodeOrNull(path) ?? throw new - InvalidOperationException( - $"Node at '{path}' does not exist in either the real or fake " + - $"subtree for '{node.Name}' member '{name}' of type " + + var potentialChild = node.GetNodeOrNull(path); + + if (potentialChild is not Node child) { + e = new InvalidOperationException( + $"AutoNode: Node at '{path}' does not exist in either the real or " + $"fake subtree for '{node.Name}' member '{name}' of type " + $"'{propertyOrField.Type.Name}'." ); + GD.PushError(e.Message); + throw e; + } // see if the unchecked node satisfies the expected type of node from the // property type - _checker.Value = godotNode; + _checker.Value = child; var originalNodeSatisfiesType = autoNode.GetScriptPropertyOrFieldType(name, _checker); if (originalNodeSatisfiesType) { // Property expected a vanilla Godot node type and it matched, so we // set it and leave. - autoNode.SetScriptPropertyOrField(name, godotNode); + autoNode.SetScriptPropertyOrField(name, child); continue; } @@ -336,25 +171,24 @@ IAutoNode autoNode // // Check to see if the node needs to be adapted to satisfy an // expected interface type. - var adaptedNode = GodotInterfaces.AdaptNode(godotNode); - _checker.Value = adaptedNode; - var adaptedNodeSatisfiesType = + var adaptedChild = GodotInterfaces.AdaptNode(child); + _checker.Value = adaptedChild; + var adaptedChildSatisfiesType = autoNode.GetScriptPropertyOrFieldType(name, _checker); - // If the adapted node does not satisfy the expected interface/adapter - // node type, then we can't connect the node to the property. - if (!adaptedNodeSatisfiesType) { - // Tell user we can't connect the node to the property. - throw new InvalidOperationException( - $"Node at '{path}' of type '{godotNode.GetType().Name}' does not " + - $"satisfy the expected type '{propertyOrField.Type.Name}' for " + - $"member '{name}' on '{node.Name}'." - ); + if (adaptedChildSatisfiesType) { + autoNode.SetScriptPropertyOrField(name, adaptedChild); + continue; } - // Otherwise, the adapted node satisfies the expected adapter or interface - // type, so we can be done. - autoNode.SetScriptPropertyOrField(name, adaptedNode); + // Tell user we can't connect the node to the property. + e = new InvalidOperationException( + $"Node at '{path}' of type '{child.GetType().Name}' does not " + + $"satisfy the expected type '{propertyOrField.Type.Name}' for " + + $"member '{name}' on '{node.Name}'." + ); + GD.PushError(e.Message); + throw e; } } diff --git a/Chickensoft.PowerUps.Tests/src/FakeNodeTree.cs b/Chickensoft.PowerUps.Tests/src/FakeNodeTree.cs deleted file mode 100644 index e7641fb..0000000 --- a/Chickensoft.PowerUps.Tests/src/FakeNodeTree.cs +++ /dev/null @@ -1,128 +0,0 @@ -#pragma warning disable -namespace Chickensoft.PowerUps; - -using System.Collections.Generic; -using System.Collections.Specialized; -using System.Linq; -using Chickensoft.GodotNodeInterfaces; -using Godot; - -public class FakeNodeTree { - // Map of node paths to FakeSceneTreeNode instances. - private readonly OrderedDictionary _nodes; - - private readonly Node _parent; - - private int _nextId; - - public FakeNodeTree( - Node parent, - Dictionary? nodes = null - ) { - _parent = parent; - _nodes = new(); - - if (nodes is Dictionary dict) { - foreach (var (path, node) in dict) { - _nodes.Add(path, node); - } - } - } - - public void AddChild(INode node) { - var name = ""; - // We use try/catch to check node name since not all node mocks may - // have stubbed the Name property. - try { - name = node.Name; - } - catch { } - - if (string.IsNullOrEmpty(name)) { - name = node.GetType().Name + "@" + _nextId++; - } - - _nodes.Add(name, node); - } - - public INode? GetNode(string path) => - _nodes.Contains(path) ? (INode)_nodes[path] : null; - - public T? GetNode(string path) where T : class, INode => - _nodes.Contains(path) ? (T)_nodes[path] : null; - - public INode? FindChild(string pattern) { - foreach (string path in _nodes.Keys) { - var node = (INode)_nodes[path]!; - var name = ""; - // We use try/catch to check node name since not all node mocks may - // have stubbed the Name property. - try { - name = node.Name; - } - catch { } - - if (!string.IsNullOrEmpty(name) && name.Match(pattern)) { - return node; - } - } - - return null; - } - - public bool HasNode(NodePath path) => _nodes.Contains((string)path); - - public INode[] FindChildren(string pattern) { - var children = new List(); - - foreach (string path in _nodes.Keys) { - var node = (INode)_nodes[path]!; - var name = ""; - try { - name = node.Name; - } - catch { } - - if (!string.IsNullOrEmpty(name) && name.Match(pattern)) { - children.Add((INode)_nodes[path]!); - } - } - - return children.ToArray(); - } - - public T? GetChild(int index) where T : class, INode { - var actualIndex = index; - if (actualIndex < 0) { - // Negative indices access the children from the last one. - actualIndex = _nodes.Count + actualIndex; - } - return _nodes[actualIndex] as T; - } - - public INode GetChild(int index) => GetChild(index); - - public int GetChildCount() => _nodes.Count; - - public INode[] GetChildren() => _nodes.Values.Cast().ToArray(); - - public void RemoveChild(INode node) { - var path = _nodes - .Keys - .Cast() - .First(k => _nodes[k] == node); - _nodes.Remove(path); - } - - public Dictionary GetAllNodes() { - var nodes = new Dictionary(); - - foreach (string path in _nodes.Keys) { - var node = (INode)_nodes[path]!; - nodes.Add(path, node); - } - - return nodes; - } -} -#pragma warning restore diff --git a/Chickensoft.PowerUps.Tests/src/IAutoNode.cs b/Chickensoft.PowerUps.Tests/src/IAutoNode.cs deleted file mode 100644 index 6bb3c30..0000000 --- a/Chickensoft.PowerUps.Tests/src/IAutoNode.cs +++ /dev/null @@ -1,217 +0,0 @@ -#pragma warning disable -namespace Chickensoft.PowerUps; - -using Chickensoft.GodotNodeInterfaces; -using Godot; -using SuperNodes.Types; - -/// -/// Interface for a PowerUp that automatically connects declared node -/// references to their corresponding instances in the scene tree. These are -/// re-implementations of Godot node manipulation and fetching methods that -/// automatically create node adapters so that you can reference Godot nodes -/// by interface. -/// -public interface IAutoNode : ISuperNode { - /// Fake node tree, used for reading fake nodes during - /// unit testing. - public FakeNodeTree? FakeNodes { get; } - - /// - /// Adds a child . Nodes can have any number of children, but every child must have a unique name. Child nodes are automatically deleted when the parent node is deleted, so an entire scene can be removed by deleting its topmost node. - /// If is true, improves the readability of the added . If not named, the is renamed to its type, and if it shares with a sibling, a number is suffixed more appropriately. This operation is very slow. As such, it is recommended leaving this to false, which assigns a dummy name featuring @ in both situations. - /// If is different than , the child will be added as internal node. Such nodes are ignored by methods like , unless their parameter include_internal is true. The intended usage is to hide the internal nodes from the user, so the user won't accidentally delete or modify them. Used by some GUI nodes, e.g. . See for available modes. - /// Note: If the child node already has a parent, the function will fail. Use first to remove the node from its current parent. For example: - /// - /// Node childNode = GetChild(0); - /// if (childNode.GetParent() != null) - /// { - /// childNode.GetParent().RemoveChild(childNode); - /// } - /// AddChild(childNode); - /// - /// If you need the child node to be added below a specific node in the list of children, use instead of this method. - /// Note: If you want a child to be persisted to a , you must set in addition to calling . This is typically relevant for tool scripts and editor plugins. If is called without setting , the newly added will not be visible in the scene tree, though it will be visible in the 2D/3D view. - /// This implementation is provided by AutoNode. Because it uses object as the type of the node, it will automatically be used instead when mixed-in with SuperNodes since that's how C# resolves things. - /// - /// The node to add as a child. - /// If true, improves the readability of the added . - /// If different than , the child will be added as internal node. - void AddChildEx( - object node, bool forceReadableName = false, Node.InternalMode @internal = Node.InternalMode.Disabled - ); - - /// - /// Finds the first descendant of this node whose name matches as in String.match. Internal children are also searched over (see internal parameter in ). - /// does not match against the full path, just against individual node names. It is case-sensitive, with "*" matching zero or more characters and "?" matching any single character except "."). - /// If is true, all child nodes are included, even if deeply nested. Nodes are checked in tree order, so this node's first direct child is checked first, then its own direct children, etc., before moving to the second direct child, and so on. If is false, only this node's direct children are matched. - /// If is true, this method only finds nodes who have an assigned . This is especially important for scenes instantiated through a script, because those scenes don't have an owner. - /// Returns null if no matching is found. - /// Note: As this method walks through all the descendants of the node, it is the slowest way to get a reference to another node. Whenever possible, consider using with unique names instead (see ), or caching the node references into variable. - /// Note: To find all descendant nodes matching a pattern or a class type, see . - /// - /// Search pattern string. - /// True recursive search. - /// True to only find nodes with the same owner. - /// The first descendant of this node whose name matches , or null. - INode? FindChildEx(string pattern, bool recursive = true, bool owned = true); - - /// - /// Finds descendants of this node whose name matches as in String.match, and/or type matches as in . Internal children are also searched over (see internal parameter in ). - /// does not match against the full path, just against individual node names. It is case-sensitive, with "*" matching zero or more characters and "?" matching any single character except "."). - /// will check equality or inheritance, and is case-sensitive. "Object" will match a node whose type is "Node" but not the other way around. - /// If is true, all child nodes are included, even if deeply nested. Nodes are checked in tree order, so this node's first direct child is checked first, then its own direct children, etc., before moving to the second direct child, and so on. If is false, only this node's direct children are matched. - /// If is true, this method only finds nodes who have an assigned . This is especially important for scenes instantiated through a script, because those scenes don't have an owner. - /// Returns an empty array if no matching nodes are found. - /// Note: As this method walks through all the descendants of the node, it is the slowest way to get references to other nodes. Whenever possible, consider caching the node references into variables. - /// Note: If you only want to find the first descendant node that matches a pattern, see . - /// - INode[] FindChildrenEx( - string pattern, string type = "", bool recursive = true, bool owned = true - ); - - /// - /// Returns a child node by its index (see ). - /// This method is often used for iterating all children of a node. - /// Negative indices access the children from the last one. - /// To access a child node via its name, use . - /// - /// - /// Child index. - /// - /// If , internal children are skipped (see internal - /// parameter in ). - /// - /// - /// The fetched node can't be casted to the given type . - /// - /// The type to cast to. Should be a descendant of . - /// - /// The child at the given index . - /// - T GetChildEx(int idx, bool includeInternal = false) where T : class, INode; - - /// - /// Returns a child node by its index (see ). This method is often used for iterating all children of a node. - /// Negative indices access the children from the last one. - /// If is false, internal children are skipped (see internal parameter in ). - /// To access a child node via its name, use . - /// - /// Child index. - /// - /// If , internal children are skipped (see internal - /// parameter in ). - /// - INode GetChildEx(int idx, bool includeInternal = false); - - /// - /// Returns a child node by its index (see ). - /// This method is often used for iterating all children of a node. - /// Negative indices access the children from the last one. - /// To access a child node via its name, use . - /// - /// - /// Child index. - /// - /// If , internal children are skipped (see internal - /// parameter in ). - /// - /// The type to cast to. Should be a descendant of . - /// - /// The child at the given index , or if not found. - /// - T? GetChildOrNullEx(int idx, bool includeInternal = false) - where T : class, INode; - - /// - /// Returns the number of child nodes. - /// If is false, internal children aren't counted (see internal parameter in ). - /// - /// - /// If , internal children are skipped (see internal - /// parameter in ). - /// - int GetChildCountEx(bool includeInternal = false); - - /// - /// Returns an array of references to node's children. - /// If is false, the returned array won't include internal children (see internal parameter in ). - /// - INode[] GetChildrenEx(bool includeInternal = false); - - /// - /// Fetches a node. The can be either a relative path (from the current node) or an absolute path (in the scene tree) to a node. If the path does not exist, null is returned and an error is logged. Attempts to access methods on the return value will result in an "Attempt to call <method> on a null instance." error. - /// Note: Fetching absolute paths only works when the node is inside the scene tree (see ). - /// Example: Assume your current node is Character and the following tree: - /// - /// /root - /// /root/Character - /// /root/Character/Sword - /// /root/Character/Backpack/Dagger - /// /root/MyGame - /// /root/Swamp/Alligator - /// /root/Swamp/Mosquito - /// /root/Swamp/Goblin - /// - /// Possible paths are: - /// - /// GetNode("Sword"); - /// GetNode("Backpack/Dagger"); - /// GetNode("../Swamp/Alligator"); - /// GetNode("/root/MyGame"); - /// - /// - /// The path to the node to return. - INode GetNodeEx(NodePath path); - - /// - /// Similar to , but does not log an error if - /// does not point to a valid . - /// - /// - /// Example: Assume your current node is Character and the following tree: - /// - /// /root - /// /root/Character - /// /root/Character/Sword - /// /root/Character/Backpack/Dagger - /// /root/MyGame - /// /root/Swamp/Alligator - /// /root/Swamp/Mosquito - /// /root/Swamp/Goblin - /// - /// Possible paths are: - /// - /// GetNode("Sword"); - /// GetNode("Backpack/Dagger"); - /// GetNode("../Swamp/Alligator"); - /// GetNode("/root/MyGame"); - /// - /// - /// - /// The path to the node to fetch. - /// The type to cast to. Should be a descendant of . - /// - /// The at the given , or if not found. - /// - T GetNodeOrNullEx(NodePath path) where T : class, INode; - - /// - /// Similar to , but does not log an error if does not point to a valid . - /// - INode? GetNodeOrNullEx(NodePath path); - - /// - /// Returns true if the node that the points to exists. - /// - /// Node path. - bool HasNodeEx(NodePath path); - - /// - /// Removes a child node. The node is NOT deleted and must be deleted manually. - /// Note: This function may set the of the removed Node (or its descendants) to be null, if that is no longer a parent or ancestor. - /// - /// Node to remove. - void RemoveChildEx(object node); -} -#pragma warning restore diff --git a/Chickensoft.PowerUps.Tests/test/fixtures/AutoDisposeTestScene.cs b/Chickensoft.PowerUps.Tests/test/fixtures/AutoDisposeTestScene.cs deleted file mode 100644 index 1329923..0000000 --- a/Chickensoft.PowerUps.Tests/test/fixtures/AutoDisposeTestScene.cs +++ /dev/null @@ -1,26 +0,0 @@ -namespace Chickensoft.PowerUps.Tests.Fixtures; - -using Chickensoft.PowerUps; -using Godot; -using SuperNodes.Types; - -[SuperNode(typeof(AutoDispose))] -public partial class AutoDisposeTestScene : Node2D { - public override partial void _Notification(int what); - - // Won't get disposed since it's read-only - public DisposableObject MyReadonlyDisposable { get; } = - new DisposableObject(); - - // Also won't get disposed since it's read-only - public DisposableObject ReadonlyDisposable => MyReadonlyDisposable; - - // Will get disposed since it's read-write - public DisposableObject MyDisposable { get; set; } = default!; - - // This never gets set, so it will be null. AutoDispose should be smart enough - // to leave it alone to avoid null reference errors. - public DisposableObject MyNullDisposable { get; set; } = default!; - - public void OnReady() => MyDisposable = new DisposableObject(); -} diff --git a/Chickensoft.PowerUps.Tests/test/fixtures/AutoDisposeTestScene.tscn b/Chickensoft.PowerUps.Tests/test/fixtures/AutoDisposeTestScene.tscn deleted file mode 100644 index 9acb81f..0000000 --- a/Chickensoft.PowerUps.Tests/test/fixtures/AutoDisposeTestScene.tscn +++ /dev/null @@ -1,6 +0,0 @@ -[gd_scene load_steps=2 format=3 uid="uid://vqs02osi5v2w"] - -[ext_resource type="Script" path="res://test/fixtures/AutoDisposeTestScene.cs" id="1_mrvkt"] - -[node name="AutoDisposeTestScene" type="Node2D"] -script = ExtResource("1_mrvkt") diff --git a/Chickensoft.PowerUps.Tests/test/fixtures/DisposableObject.cs b/Chickensoft.PowerUps.Tests/test/fixtures/DisposableObject.cs deleted file mode 100644 index 53f9f8d..0000000 --- a/Chickensoft.PowerUps.Tests/test/fixtures/DisposableObject.cs +++ /dev/null @@ -1,12 +0,0 @@ -namespace Chickensoft.PowerUps.Tests.Fixtures; - -using System; - -public class DisposableObject : IDisposable { - public bool IsDisposed { get; private set; } - - public void Dispose() { - IsDisposed = true; - GC.SuppressFinalize(this); - } -} diff --git a/Chickensoft.PowerUps.Tests/test/src/AutoDisposeTest.cs b/Chickensoft.PowerUps.Tests/test/src/AutoDisposeTest.cs deleted file mode 100644 index 4a04bca..0000000 --- a/Chickensoft.PowerUps.Tests/test/src/AutoDisposeTest.cs +++ /dev/null @@ -1,72 +0,0 @@ -namespace Chickensoft.PowerUps.Tests; - -using System; -using System.Collections.Generic; -using System.Collections.Immutable; -using System.Threading.Tasks; -using Chickensoft.GoDotTest; -using Chickensoft.PowerUps.Tests.Fixtures; -using Godot; -using GodotTestDriver; -using Shouldly; -using SuperNodes.Types; - -public class AutoDisposeTest : TestClass { - private Fixture _fixture = default!; - private AutoDisposeTestScene _scene = default!; - - public AutoDisposeTest(Node testScene) : base(testScene) { } - - [Setup] - public async Task Setup() { - _fixture = new Fixture(TestScene.GetTree()); - _scene = await _fixture.LoadAndAddScene( - autoFree: false, - autoRemoveFromRoot: false - ); - } - - [Test] - public async Task DisposesWriteableProperties() { - _scene.MyReadonlyDisposable.IsDisposed.ShouldBeFalse(); - _scene.ReadonlyDisposable.IsDisposed.ShouldBeFalse(); - _scene.MyDisposable.IsDisposed.ShouldBeFalse(); - _scene.MyNullDisposable.ShouldBeNull(); - - _scene.GetParent().RemoveChild(_scene); - - _scene.MyReadonlyDisposable.IsDisposed.ShouldBeFalse(); - _scene.ReadonlyDisposable.IsDisposed.ShouldBeFalse(); - _scene.MyDisposable.IsDisposed.ShouldBeTrue(); - _scene.MyNullDisposable.ShouldBeNull(); - - await _fixture.Cleanup(); - - // Verify some edge cases: - - // Check register method does nothing if not readable. - var property = new ScriptPropertyOrField( - Name: "name", - Type: typeof(object), - IsField: false, - IsMutable: true, - IsReadable: false, - Attributes: ImmutableDictionary< - string, ImmutableArray - >.Empty - ); - - var members = new Dictionary { - ["name"] = property - }.ToImmutableDictionary(); - - Should.NotThrow(() => - Disposer.RegisterDisposables(members, new HashSet(), null!) - ); - - // Check disposer does nothing if not disposable. - Should.NotThrow(() => - Disposer.DisposeDisposables(new HashSet { null! }) - ); - } -} diff --git a/Chickensoft.PowerUps.Tests/test/src/AutoNodeInvalidCastTest.cs b/Chickensoft.PowerUps.Tests/test/src/AutoNodeInvalidCastTest.cs index b966e5c..b7eef3a 100644 --- a/Chickensoft.PowerUps.Tests/test/src/AutoNodeInvalidCastTest.cs +++ b/Chickensoft.PowerUps.Tests/test/src/AutoNodeInvalidCastTest.cs @@ -1,8 +1,11 @@ namespace Chickensoft.PowerUps.Tests; +using System; +using Chickensoft.GodotNodeInterfaces; using Chickensoft.GoDotTest; using Chickensoft.PowerUps.Tests.Fixtures; using Godot; +using Moq; using Shouldly; public class AutoNodeInvalidCastTest : TestClass { @@ -19,4 +22,14 @@ public void ThrowsOnIncorrectNodeType() { var node = scene.Instantiate(); node.Node.ShouldBeNull(); } + + [Test] + public void ThrowsIfFakedChildNodeIsWrongType() { + var scene = new AutoNodeInvalidCastTestScene(); + scene.FakeNodeTree(new() { ["Node3D"] = new Mock().Object }); + + Should.Throw( + () => scene._Notification((int)Node.NotificationSceneInstantiated) + ); + } } diff --git a/Chickensoft.PowerUps/Chickensoft.PowerUps.csproj b/Chickensoft.PowerUps/Chickensoft.PowerUps.csproj index 5e9cfa8..e4ba0d8 100644 --- a/Chickensoft.PowerUps/Chickensoft.PowerUps.csproj +++ b/Chickensoft.PowerUps/Chickensoft.PowerUps.csproj @@ -46,7 +46,7 @@ - + diff --git a/README.md b/README.md index e09a3f5..47816ce 100644 --- a/README.md +++ b/README.md @@ -2,8 +2,6 @@ [![Chickensoft Badge][chickensoft-badge]][chickensoft-website] [![Discord][discord-badge]][discord] [![Read the docs][read-the-docs-badge]][docs] ![line coverage][line-coverage] - - A collection of power-ups for your C# Godot game scripts that work with the [SuperNodes] source generator. --- @@ -12,10 +10,10 @@ A collection of power-ups for your C# Godot game scripts that work with the [Sup Chickensoft.PowerUps

-Currently, two PowerUps are provided by this package: `AutoNode` and `AutoDispose`. +Currently, two PowerUps are provided by this package: `AutoNode` and `AutoSetup`. - ๐ŸŒฒ AutoNode: automatically connect fields and properties to their corresponding nodes in the scene tree โ€” also provides access to nodes via their interfaces using [GodotNodeInterfaces]. -- ๐Ÿšฎ AutoDispose: automatically dispose of disposable properties owned by your script when it exits the scene tree. +- ๐Ÿ›  AutoSetup: provides a mechanism for late, two-phase initialization in Godot node scripts to facilitate unit-testing. > Chickensoft also maintains a third PowerUp for dependency injection called `AutoInject` that resides in its own [AutoInject] repository. @@ -31,8 +29,8 @@ To use the PowerUps, add the following to your `.csproj` file. Be sure to get th - - + + ``` @@ -73,23 +71,6 @@ public partial class MyNode : Node2D { } ``` -### ๐Ÿคฏ Using Interfaces - -If you're using [GodotNodeInterfaces], you can access and manipulate descendent nodes via their interface instead of their concrete Godot type. Each AutoNode receives a number of additional methods that match a Godot method, but with the added "Ex" suffix (short for Extended). - -- `AddChildEx()` -- `FindChildEx()` -- `FindChildrenEx()` -- `GetChildEx()` -- `GetChildrenEx()` -- `GetChildOrNullEx()` -- `GetChildCountEx()` -- `GetChildrenEx()` -- `GetNodeEx()` -- `GetNodeOrNullEx()` -- `HasNodeEx()` -- `RemoveChildEx()` - ### ๐Ÿงช Testing We can easily write a test for the example above by substituting mock nodes: @@ -150,45 +131,41 @@ public class MyNodeTest : TestClass { } ``` -## ๐Ÿšฎ AutoDispose +## ๐Ÿ›  AutoSetup -The `AutoDispose` PowerUp will automatically dispose of writeable properties on your script that implement `IDisposable` but do not inherit `Godot.Node` when the node exits the scene tree, preventing the need for manually cleaning up disposable, non-node objects that your script owns. - -AutoDispose was designed to work nicely with the dependency injection system from [AutoInject] and disposable objects, like the bindings from [LogicBlocks]. +The `AutoSetup` will conditionally call the `void Setup()` method your node script has if from `_Ready` if (and only if) the `IsTesting` field it adds to your node is false. Conditionally calling a setup method allows you to split your node's late member initialization into two-phases, allowing nodes to be unit tested. If writing tests for your node, simply initialize any members that would need to be mocked in a test in your `Setup()` method. ```csharp -using Chickensoft.AutoInject; using Chickensoft.PowerUps; using Godot; using SuperNodes.Types; -// Note that Dependent is a PowerUp provided by AutoInject. - -[SuperNode(typeof(Dependent), typeof(AutoDispose))] +[SuperNode(typeof(AutoSetup))] public partial class MyNode : Node2D { public override partial void _Notification(int what); - // Use AutoInject to lookup the nearest SomeDisposable object provided above - // us in the scene tree. - // - // Since this is a read-only property, it will not have Dispose called on it - // by AutoDispose when we exit the scene tree. This is desirable since this - // script doesn't own this object โ€” it just needs to use it. - [Dependency] - public SomeDisposable DisposableDependency => DependOn(); - - // This object will automatically have Dispose called on it by AutoDispose - // when we exit the scene tree since it is a writeable property (which tells - // AutoDispose this script owns it and should dispose of it). - public MyDisposable MyDisposableObject { get; set; } = default!; - - // Even though Godot nodes are disposable, this won't be disposed since it - // inherits from Godot.Node. Godot manages node references automatically. - public Node2D OtherNode { get; set; } = default!; - - // ... + public MyObject Obj { get; set; } = default!; + + public void Setup() { + // Setup is called from the Ready notification if our IsTesting property + // (added by AutoSetup) is false. + + // Initialize values which would be mocked in a unit testing method. + Obj = new MyObject(); + } + + public void OnReady() { + // Guaranteed to be called after Setup() + + // Use object we setup in Setup() method (or, if we're running in a unit + // test, this will use whatever the test supplied) + Obj.DoSomething(); + } +} ``` +> ๐Ÿ’ก [AutoInject] provides this functionality out-of-the-box for nodes that also need late, two-phase initialization. It also supplies an `IsTesting` property but will call the `Setup()` method after dependencies have been resolved (but before `OnResolved() is called`). If you're using AutoInject, note that you can either use the `AutoSetup` or `Dependent` PowerUp on a node script, but not both. + --- ๐Ÿฃ Package generated from a ๐Ÿค Chickensoft Template โ€” @@ -204,7 +181,6 @@ public partial class MyNode : Node2D { [SuperNodes]: https://github.com/chickensoft-games/SuperNodes [AutoInject]: https://github.com/chickensoft-games/AutoInject -[LogicBlocks]: https://github.com/chickensoft-games/LogicBlocks [Nuget]: https://www.nuget.org/packages?q=Chickensoft [unique-nodes]: https://docs.godotengine.org/en/stable/tutorials/scripting/scene_unique_nodes.html diff --git a/global.json b/global.json index 328008a..ab3074e 100644 --- a/global.json +++ b/global.json @@ -1,9 +1,9 @@ { "sdk": { - "version": "8.0.100-rc.1.23463.5", + "version": "8.0.100-rc.2.23502.2", "rollForward": "latestMinor" }, "msbuild-sdks": { - "Godot.NET.Sdk": "4.2.0-beta.1" + "Godot.NET.Sdk": "4.2.0-beta.5" } }