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

Engine: replace a "plugin return value" hack with something appropriate #2403

Draft
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

ivan-mogilko
Copy link
Contributor

@ivan-mogilko ivan-mogilko commented May 2, 2024

Necessary for #2313 (indirectly).

There is this very old hack which I invented while porting script VM to 64-bit back in 2012. When I've been rewriting script VM to work with RuntimeScriptValue struct, instead of a plain int32, plugins were becoming an issue, because there was no way to change the return values of their functions without modifying plugin API (and each existing plugin). And if we keep plugin functions cast to something returning an int, then we cannot know if that's a pointer, and which kind of a pointer that is (which exactly dynamic object type).

Back then I introduced a "temporary" hack: have a GlobalReturnValue object, which is assigned whenever plugin does anything related to the managed objects: create, register, resolve a handle, add reference, and so forth. Then hope that it is the value that the plugin is intending to return from its function. Silly as it is, this naive solution actually worked, for the actively used plugins at least. That's why it was forgotten about and never replaced.

But of course this is not a correct solution, simply because a plugin does not have to return a dynamic object that it touched last, it may return something completely different, previously saved instead.

The new solution I'm trying here is based on a simple but more logical assumption:

  1. A plugin's return value is either a primitive type (int or float) or a memory address.
  2. If it's a memory address, then this must be a managed object's address, as AGS script does not support plain pointers returned from a function (at least as of now).
  3. If it's a managed object, then we may quickly search for it in the dynamic memory manager (it has a hashtable that resolves addresses to handles).

Therefore, we try the plugin function's return value as a address, and ask for its respective IScriptObject (dynamic object manager). If we succeeded, then we may assign a RuntimeScriptValue as a "Script Object". If we fail, we assign one as a "Plugin Argument", which may be anything else.


TEST LIST:

  • Win x32 engine
  • Win x64 engine
  • Linux x64 engine
  • OSX x64 engine
  • Android
  • Performance test

This is a very old hack which I invented while porting script VM to 64-bit back in 2012.
When I've been rewriting script VM to work with RuntimeScriptValue struct, instead of a plain int32, plugins were becoming an issue, because there was no way to change the return values of their functions without modifying plugin API (and each existing plugin). And if we keep plugin functions cast to something returning an int, then we cannot know if that's a pointer, and which kind of a pointer that is (which exactly dynamic object type).

Back then I introduced a "temporary" hack: have a GlobalReturnValue object, which is assigned whenever plugin does *anything* related to the managed objects: create, register, resolve a handle, add reference, and so forth. Then *hope* that it is the value that the plugin is intending to return from its function.
Silly as it is, this naive solution actually worked, for the actively used plugins at least. That's why it was forgotten about and never replaced.

But of course this is simply not a correct solution, simply because a plugin does not have to return a dynamic object that it touched last, it may return something completely different, previously saved instead.

The new solution I'm trying here is based on a simple but more logical assumption:
1. A plugin's return value is either a primitive type (int or float) or a memory address.
2. If it's a memory address, then this must be a managed object's address, as AGS script does not support plain pointers (at least as of now).
3. If it's a managed object, then we may quickly search for it in the dynamic memory manager (it has a hashtable that resolves addresses to handles).

Therefore, we try the plugin function's return value as a address, and ask for its respective IScriptObject (dynamic object manager).
If we succeeded, then we may assign a RuntimeScriptValue as a "Script Object".
If we fail, we assign one as a "Plugin Argument", which may be anything else.
@ivan-mogilko ivan-mogilko marked this pull request as draft May 2, 2024 20:52
@ericoporto
Copy link
Member

I don't understand most of this but if the issue is testing in other platforms I can test, but I need to be told how to test this - I can build and run things on them, but I don't know if there is any specific here beyond "plugin that already works keeps working".

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

Successfully merging this pull request may close these issues.

2 participants