From d8bfeaa4e1f11b38e7249ea3965d0dfda7d11700 Mon Sep 17 00:00:00 2001
From: Joseph Cloutier <player3.14@gmail.com>
Date: Sun, 23 Jun 2024 13:26:20 -0400
Subject: [PATCH] Fix behavior of `@:update` listeners with only optional
 arguments.

---
 src/echoes/macro/SystemBuilder.hx | 25 +++++++++++++------------
 test/EdgeCaseTest.hx              | 26 ++++++++++++++++++++++++++
 2 files changed, 39 insertions(+), 12 deletions(-)

diff --git a/src/echoes/macro/SystemBuilder.hx b/src/echoes/macro/SystemBuilder.hx
index 3d8e0d2..2572c79 100644
--- a/src/echoes/macro/SystemBuilder.hx
+++ b/src/echoes/macro/SystemBuilder.hx
@@ -545,7 +545,7 @@ abstract ListenerFunction(ListenerFunctionData) from ListenerFunctionData {
 				kind: FFun({
 					args: args,
 					ret: macro:Void,
-					expr: call()
+					expr: call(macro entity, macro __dt__)
 				}),
 				pos: this.pos
 			};
@@ -559,21 +559,21 @@ abstract ListenerFunction(ListenerFunctionData) from ListenerFunctionData {
 	 * `entity`, and any required components, so it's important to ensure all of
 	 * these values are available in the current context.
 	 */
-	private function call():Expr {
+	private function call(getEntity:Expr, getDeltaTime:Expr):Expr {
 		final args:Array<Expr> = [for(arg in this.args) {
 			switch(arg.type.followComplexType()) {
 				case macro:StdTypes.Float:
 					//Defined as a private variable of `System`.
-					macro __dt__;
+					getDeltaTime;
 				case macro:echoes.Entity:
 					//Defined as a wrapper function's first argument, and also
 					//defined in `callDuringUpdate()`.
-					macro entity;
+					getEntity;
 				default:
 					if(arg.opt || arg.value != null) {
 						//Look up the optional component's value. (May be null
 						//and that's fine.)
-						EntityTools.get(macro entity, arg.type.followComplexType());
+						EntityTools.get(getEntity, arg.type.followComplexType());
 					} else {
 						//Defined as one of the wrapper function's arguments.
 						macro $i{ arg.name };
@@ -591,21 +591,22 @@ abstract ListenerFunction(ListenerFunctionData) from ListenerFunctionData {
 		if(components.length > 0) {
 			//Iterate over a `View`'s entities.
 			return macro $view.iter($wrapper);
+		} else if(optionalComponents.length > 0) {
+			return macro for(entity in echoes.Echoes.activeEntities)
+				${ call(macro entity, macro __dt__) };
 		} else {
 			//No components to filter by, but there may still be an `Entity`
 			//argument. (And/or a `Float` argument, which isn't relevant.)
 			for(arg in this.args) {
-				switch(arg.type.followComplexType()) {
-					case macro:echoes.Entity:
-						//Iterate over all entities.
-						return macro for(entity in echoes.Echoes.activeEntities)
-							${ call() };
-					default:
+				if(arg.type.followComplexType().match(macro:echoes.Entity)) {
+					//Iterate over all entities.
+					return macro for(entity in echoes.Echoes.activeEntities)
+						${ call(macro entity, macro __dt__) };
 				}
 			}
 			
 			//Don't iterate over anything.
-			return call();
+			return call(macro throw "Unable to select an entity because this function has no required components", macro __dt__);
 		}
 	}
 }
diff --git a/test/EdgeCaseTest.hx b/test/EdgeCaseTest.hx
index b9fe5ed..1999b55 100644
--- a/test/EdgeCaseTest.hx
+++ b/test/EdgeCaseTest.hx
@@ -22,6 +22,23 @@ class EdgeCaseTest extends Test {
 	
 	//Tests may be run in any order, but not in parallel.
 	
+	private function testAllArgumentsOptional():Void {
+		new OptionalListenerSystem().activate();
+		Echoes.update();
+		assertTimesCalled(0, "OptionalListenerSystem.optionalNameUpdated");
+		
+		final entity:Entity = new Entity();
+		entity.add(("name":Name));
+		Echoes.update();
+		assertTimesCalled(1, "OptionalListenerSystem.optionalNameUpdated");
+		
+		//When there are multiple entities, the function should be called for
+		//each, whether or not they have `Name` components.
+		new Entity();
+		Echoes.update();
+		assertTimesCalled(3, "OptionalListenerSystem.optionalNameUpdated");
+	}
+	
 	private function testChildSystems():Void {
 		new NameSubsystem().activate();
 		
@@ -341,3 +358,12 @@ class RecursiveEventSystem extends System implements IMethodCounter {
 		entity.add(permanent);
 	}
 }
+
+class OptionalListenerSystem extends System implements IMethodCounter {
+	@:update private function optionalNameUpdated(?name:Name):Void {}
+	
+	//These aren't allowed, and would throw compile errors, preventing the tests
+	//from running at all.
+	//@:add private function optionalNameAdded(?name:Name):Void {}
+	//@:remove private function optionalNameRemoved(?name:Name):Void {}
+}