Replies: 1 comment 1 reply
-
The methods being stored in an array is intentional, due to the fact that more than one method can share the same name in one class. I have however updated NoReflect to include a prebuilt method map builder function that takes in a class and returns an object mapping method name to method object. |
Beta Was this translation helpful? Give feedback.
1 reply
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
-
Hi guys I have one more suggestion to try and make what you’re trying to do here work a bit better.
This discrepancy might seem insignificant on its own but if you look at the “tracers.js” example mod, the current recommended way to use NoReflect is to define helper functions like
mathHelperFunction(name, args)
to use for calling the exported NoReflect functions, which are implemented in a way that iterates over the ENTIRE list of exported functions in the NoReflect class each time to find it first in order to call it.This is the worst possible way it could’ve been implemented for performance, the browser isn’t smart enough to see what you’re trying to do here and optimize it, it will iterate over every element in the list and attempt to compare the name of every function regardless if the function you’re searching for is the first or very last on the list before returning and actually calling it. This process is then repeated every time the function needs to be called again.
You need to change it so “methods” is an object (
{}
) instead of a list ([]
) and indexes all the methods into it by method name. That way when you go to call a method by name, you can just domethods.methodNameHere.exec(args)
and the underlying JavaScript engine will automatically compute a hashtable of all the names of the functions in the NoReflect class to be able search it for the right functions orders of magnitude faster than the current implementation can. This also completely eliminates the need for the helper functions in the “tracers.js” making it easier to work with.I hope you understand my concerns, you’d have to break compatibility with existing mods in order to fix this so I thought it’s better to point it out sooner than later.
Beta Was this translation helpful? Give feedback.
All reactions