Skip to content
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

Reflect.compareMethods bug ( HashLink 1.10.0) #301

Closed
m0rkeulv opened this issue Sep 9, 2019 · 2 comments
Closed

Reflect.compareMethods bug ( HashLink 1.10.0) #301

m0rkeulv opened this issue Sep 9, 2019 · 2 comments

Comments

@m0rkeulv
Copy link
Member

m0rkeulv commented Sep 9, 2019

using Reflect.compareMethods with "callback" variables when variable signature is dynamic and method signature is something else causes the compareMethods to return false even though the methods are the same.

(This is breaking openFLs event system)

Run this example and you will see that callbackA and callbackC both points to testMethodX but when compared with compareMethods the result is false.

package;

class Main
{
	static var callbackA:Dynamic->String;
	static var callbackB:Dynamic->String;
	static var callbackC:Dynamic->String;
	static var callbackD:Dynamic->String;

	static function testMethodX(val:Int):String
	{
		return "X";
	}

	static function testMethodY(val:Dynamic):String
	{
		return "Y";
	}

	static function main()
	{
		callbackA = testMethodX;
		callbackB = testMethodY;
		callbackC = testMethodX;
		callbackD = testMethodY;
		trace("-- compare with the same variable --");
		trace("callback A == calback A :" + Reflect.compareMethods(callbackA, callbackA) + " (Expected true)");
		trace("callback B == calback B :" + Reflect.compareMethods(callbackB, callbackB) + " (Expected true)");
		
		trace("");
		trace("-- compare diffrent variables and different methods --");
		trace("callback A == calback B :" + Reflect.compareMethods(callbackA, callbackB) + " (Expected false)");
		trace("callback B == calback A :" + Reflect.compareMethods(callbackB, callbackA) + " (Expected false)");
		
		trace("");
		trace("-- compare same method diffrent variables --");
		trace("callback A == calback C :" + Reflect.compareMethods(callbackA, callbackC) + " (Expected true) <-- this one returns the wrong value");
		trace("callback B == calback D :" + Reflect.compareMethods(callbackB, callbackD) + " (Expected true)");
		trace("");

		trace("callback A and C is the same method however it returns false");
		trace("The only diffrence from B and D is that the method signature is not dynamic, while the callback signature is");
		
		trace("sanity check");
		trace(callbackA(1) + " == " +callbackC(1));
		trace(callbackB(1) + " == " +callbackD(1));
	}
}

For some reason a normal Reflect.compare seems to work as expected, so this is aworkaround for now.

trace("-- compare with the same variable --");
trace("callback A == calback A :" + (Reflect.compare(callbackA, callbackA) == 0) + " (Expected true)");
trace("callback B == calback B :" + (Reflect.compare(callbackB, callbackB) == 0) + " (Expected true)");

trace("");
trace("-- compare diffrent variables and different methods --");
trace("callback A == calback B :" + (Reflect.compare(callbackA, callbackB) == 0)+ " (Expected false)");
trace("callback B == calback A :" + (Reflect.compare(callbackB, callbackA) == 0) + " (Expected false)");

trace("");
trace("-- compare same method diffrent variables --");
trace("callback A == calback C :" + (Reflect.compare(callbackA, callbackC) == 0) + " (Expected true)");
trace("callback B == calback D :" + (Reflect.compare(callbackB, callbackD) == 0) + " (Expected true)");
@ncannasse
Copy link
Member

I think this is related to Haxe/HL code generator generating a custom wrapper for the assignments
callbackA = testMethodX and callbackC = testMethodY, which generates not equal functions. We should be able to tag somehow these methods as wrappers so we can go through them in Reflect.compareMethods, but the bytecode does not carry this information atm.

However, as a temporary workaround I think that doing the following should work:

callbackA = (testMethodX : Dynamic);
callbackC = (testMethodX : Dynamic);

@ncannasse
Copy link
Member

See #578

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants