Added in basic scripting features to Dolphin #12084

Lobsterzelda · 2023-08-01T20:06:16Z

This PR adds in basic scripting features to Dolphin.

It removes a lot of the features in my original scripting branch PR, in order to make the code easier to review (which will be added back in in future PRs).

Lobsterzelda · 2023-08-01T20:34:45Z

@phire , @AdmiralCurtiss , @Pokechu22 , @JosJuice , @Hibyehello , @lioncash

This is a simplified version of my original large PR from here: #11540

This branch is ready to be tested/merged right now. It designs+implements the APIs for the scripting DLLs to interact with Dolphin, creates the helper classes that are used by scripts, contains the code to handle running script callbacks (ex. OnInstructionHitCallbacks), and contains the code to initialize scripts from a DLL. Additionally, it also contains the Script Manager Window in order to start running scripts.

BhaaLseN

Well, guess this ended up being a mostly code-style based review for now.

Theres a whole lot going on, and I think I need to do another pass with a clear head for the nitty-gritty parts.

Some things look odd at first glance, and might make more sense in the big picture, so I tried to not point out all of them here (and, tbf., I stopped looking at the other PR at some point because it just went waaaay out of hand in the number of changes - but for a feature that size, I suppose thats expected)

Source/Core/Core/Core.cpp

Source/Core/Core/HW/CPU.cpp

Source/Core/Core/HW/SI/SI_DeviceGCController.cpp

Source/Core/Core/PowerPC/MMU.cpp

Source/Core/Core/Scripting/APIModules/EmuAPI.cpp

Source/Core/Core/Scripting/HelperClasses/GCButton_API_Implementations.cpp

Source/Core/Core/Scripting/HelperClasses/InstructionBreakpointsHolder.cpp

Source/Core/Core/Scripting/HelperClasses/MemoryAddressBreakpointsHolder.cpp

Source/Core/Core/Scripting/HelperClasses/VersionResolver.h

Lobsterzelda · 2023-08-03T02:32:04Z

@BhaaLseN I cut out the EventCallbackRegistration directory entirely, removed EmuAPI, removed most references to Scripts from outside of the Script directory, and replaced some of the functions in ScriptUtilities.cpp with TODOs in order to cut down on the amount of code in this PR.

This has resulted in the PR going from about 7,800 changed lines to about 4,800 changed lines.

I'm hopeful that this will make reviewing the PR a bit more manageable.

BhaaLseN · 2023-08-05T10:38:21Z

It certainly helps, but wasn't my intention to have you cut things out; i mainly meant that I need another pass over it once I cleared my head. Which is hopefully this weekend, but no promises.
But if it makes sense as a smaller PR, by all means go for it.

Lobsterzelda · 2023-08-07T02:36:49Z

It certainly helps, but wasn't my intention to have you cut things out; i mainly meant that I need another pass over it once I cleared my head. Which is hopefully this weekend, but no promises. But if it makes sense as a smaller PR, by all means go for it.

It's all good. I was planning to make the PR smaller anyways.

@BhaaLseN Btw, I finished making all of the changes you suggested.

Source/Core/Core/Scripting/HelperClasses/FileUtility_API_Implementations.cpp

Source/Core/Core/Scripting/HelperClasses/FileUtility_API_Implementations.h

Source/Core/Core/Scripting/HelperClasses/FunctionMetadata.h

Source/Core/DolphinQt/ScriptManager.cpp

lioncash · 2023-08-09T01:07:36Z

Source/Core/Core/Scripting/CoreScriptInterface/InternalScriptAPIs/ArgHolder_APIs.h

+  void* (*CreateBoolArgHolder)(int);
+
+  // Creates + returns an unsigned_8 ArgHolder* with the specified value.
+  void* (*CreateU8ArgHolder)(unsigned long long);


maybe just use uint64_t, etc for the relevant functions?

For all of the function pointers which are going to be passed across the DLL boundary (such as this one), I tried to make all integer types in the signature/parameters either unsigned char, signed char, unsigned long long, and signed long long. This way, I could be sure that the data I'm passing would be either 1 byte or the maximum bytesize for the system (realistically, 8 bytes for any computer running Dolphin).

Also, I want to make sure that the type exists in plain C, which is why I tried to avoid including other types (ex. I return ints from functions instead of bool, since C doesn't have a bool type).

Sorry to necro but one thing is that using variable length types I think causes annoying ABI concerns (especially across languages that may not have ULL as an inbuilt type), While ULL (unsigned long long) is guaranteed to be 64 bits or larger, this makes things incompatible if Dolphin is rebuilt and ULL changes.

And realistically if something doesn't have uint64_t, it's already a super esoteric implementation of C in the first place.

lioncash · 2023-08-09T01:09:42Z

Source/Core/Core/Scripting/CoreScriptInterface/InternalScriptAPIs/ArgHolder_APIs.h

+  void* (*CreateBytesArgHolder)();
+
+  // Takes a std::vector<s16> ArgHolder* as input, and returns the number of bytes in the list.
+  unsigned long long (*GetSizeOfBytesArgHolder)(void*);


Suggested change

unsigned long long (*GetSizeOfBytesArgHolder)(void*);

size_t (*GetSizeOfBytesArgHolder)(void*);

Source/Core/Core/Scripting/HelperClasses/FunctionMetadata.cpp

Lobsterzelda · 2023-08-11T03:23:32Z

@lioncash I added in all of the fixes you suggested, and marked them as resolved. The only 2 changes I didn't do were the ones in ArgHolder_APIs.h, since I want to keep those functions C-compatible.

Lobsterzelda · 2023-09-04T04:56:56Z

I updated the PR to make it even shorter, so that it would be easier to read. I also added 2 new README type files inside of the Source/Core/Core/Scripting folder:

https://github.com/Lobsterzelda/lobster-zelda-dolphin/blob/bare-bones-scripting-branch/Source/Core/Core/Scripting/ScriptingPlugins_Documentation.md

ScriptingPlugins_Documentation.md describes how to create a new ScriptingPlugin (to add in support for a new Scripting language), and describes what functions a DLL need to export/implement. It also describes the general process for adding in a new scripting language.

https://github.com/Lobsterzelda/lobster-zelda-dolphin/blob/bare-bones-scripting-branch/Source/Core/Core/Scripting/Common_Scripting_Functions_Documentation.md

Common_Scripting_Functions_Documentation.md describes what the purpose is behind each file in this PR.

Both of these files are about 200 lines, so I'm hoping that this short documentation will make it easier to read+understand the components of this PR (which will hopefully lead to this PR being merged).

@phire , @Pokechu22 , @AdmiralCurtiss , @Hibyehello , @lioncash , @BhaaLseN , @JosJuice , @CasualPokePlayer

BhaaLseN

I just skimmed over this; and I generally prefer identifiers to be marked up (or rather, down, using single backticks `); but I also see that this might be quite the effort, so I won't insist on getting that done to the two markdown files.

BhaaLseN · 2023-09-04T16:53:15Z

Source/Core/Core/Scripting/HelperClasses/InstructionBreakpointsHolder.cpp

+
+bool InstructionBreakpointsHolder::ContainsBreakpoint(unsigned int addr) const
+{
+  return (std::count(breakpoint_addresses.begin(), breakpoint_addresses.end(), addr) > 0);


I wonder if this is better suited as a std::unordered_set (or std::set) so we can just use the contains method that was added with C++20...unless we still have platforms that don't properly support C++20. At least you wouldn't have to worry about duplicates either that way, and it could clean up removal since you can just pass in the value.

C++20 should be fine nowadays.

BhaaLseN · 2023-09-04T17:07:13Z

Source/Core/Core/Scripting/ScriptingPlugins_Documentation.md

+/ (D)
+
+----PluginSources (D)
+
+--------Lua (D)
+
+------------extensions.txt (F)
+
+------------Source (D)
+
+----------------CoreScriptInterface (D)
+
+----------------<The source code for the DLL> (Multiple files and or directories).


This seems difficult to read, how about making this a code block instead?

Suggested change

/ (D)

----PluginSources (D)

--------Lua (D)

------------extensions.txt (F)

------------Source (D)

----------------CoreScriptInterface (D)

----------------<The source code for the DLL> (Multiple files and or directories).

[/]

+- [PluginSources]

+- [Lua]

+- extensions.txt

+- [Source]

+- [CoreScriptInterface]

+- <The source code for the DLL> (Multiple files and or directories).

Not really feeling that one either, but maybe we can come up with something more readable together.

twitnic · 2023-10-18T00:59:18Z

Any Update for this PR? Lua Support / a art of scripting would be really nice ...

Lobsterzelda · 2023-10-22T03:24:35Z

Any Update for this PR? Lua Support / a art of scripting would be really nice ...

Basically, the only person with both the time and scripting knowledge/experience to review this MR would be @phire

However, Phire hasn't been online for the last 6 or so months, so it's unclear when this MR will be reviewed+approved.

TryTwo · 2023-11-24T22:36:47Z

Is there a full build with python that I can mess around with somewhere?

Lobsterzelda · 2023-12-02T06:50:30Z

Is there a full build with python that I can mess around with somewhere?

Yes. You can use this project code: https://github.com/Lobsterzelda/lobster-zelda-dolphin/tree/lua-support

InputEvelution · 2024-12-03T07:14:17Z

Basically, the only person with both the time and scripting knowledge/experience to review this MR would be @phire
However, Phire hasn't been online for the last 6 or so months, so it's unclear when this MR will be reviewed+approved.

Any update on this? It would be a shame if this never got merged because a single Dolphin developer isn't active enough to look at it...

Lobsterzelda force-pushed the bare-bones-scripting-branch branch 2 times, most recently from 9d856f9 to 823ef26 Compare August 1, 2023 20:14

Lobsterzelda mentioned this pull request Aug 1, 2023

Adding in Scripting Support #11540

Closed

Lobsterzelda force-pushed the bare-bones-scripting-branch branch 5 times, most recently from 205c18d to 37fc3e8 Compare August 2, 2023 02:26

BhaaLseN reviewed Aug 2, 2023

View reviewed changes

Lobsterzelda force-pushed the bare-bones-scripting-branch branch 5 times, most recently from aae4bb1 to 25ccc70 Compare August 3, 2023 02:26

Lobsterzelda force-pushed the bare-bones-scripting-branch branch from 25ccc70 to 1d6f0a3 Compare August 7, 2023 02:17

lioncash reviewed Aug 9, 2023

View reviewed changes

Lobsterzelda force-pushed the bare-bones-scripting-branch branch 3 times, most recently from 796df75 to 3eaa045 Compare August 11, 2023 03:08

Lobsterzelda force-pushed the bare-bones-scripting-branch branch 4 times, most recently from 03f2389 to de51495 Compare September 4, 2023 02:49

Added in basic scripting features to Dolphin

ab78787

Lobsterzelda force-pushed the bare-bones-scripting-branch branch from de51495 to ab78787 Compare September 4, 2023 03:01

BhaaLseN reviewed Sep 4, 2023

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Added in basic scripting features to Dolphin #12084

Added in basic scripting features to Dolphin #12084

Lobsterzelda commented Aug 1, 2023

Lobsterzelda commented Aug 1, 2023

BhaaLseN left a comment

Lobsterzelda commented Aug 3, 2023

BhaaLseN commented Aug 5, 2023

Lobsterzelda commented Aug 7, 2023

lioncash Aug 9, 2023

Lobsterzelda Aug 9, 2023

ElectrifiedStrawberry May 8, 2024

lioncash Aug 9, 2023

Lobsterzelda commented Aug 11, 2023

Lobsterzelda commented Sep 4, 2023

BhaaLseN left a comment

BhaaLseN Sep 4, 2023

JosJuice Sep 4, 2023

BhaaLseN Sep 4, 2023

twitnic commented Oct 18, 2023

Lobsterzelda commented Oct 22, 2023

TryTwo commented Nov 24, 2023

Lobsterzelda commented Dec 2, 2023

InputEvelution commented Dec 3, 2024

	unsigned long long (GetSizeOfBytesArgHolder)(void);
	size_t (GetSizeOfBytesArgHolder)(void);

Added in basic scripting features to Dolphin #12084

Are you sure you want to change the base?

Added in basic scripting features to Dolphin #12084

Conversation

Lobsterzelda commented Aug 1, 2023

Lobsterzelda commented Aug 1, 2023

BhaaLseN left a comment

Choose a reason for hiding this comment

Lobsterzelda commented Aug 3, 2023

BhaaLseN commented Aug 5, 2023

Lobsterzelda commented Aug 7, 2023

lioncash Aug 9, 2023

Choose a reason for hiding this comment

Lobsterzelda Aug 9, 2023

Choose a reason for hiding this comment

ElectrifiedStrawberry May 8, 2024

Choose a reason for hiding this comment

lioncash Aug 9, 2023

Choose a reason for hiding this comment

Lobsterzelda commented Aug 11, 2023

Lobsterzelda commented Sep 4, 2023

BhaaLseN left a comment

Choose a reason for hiding this comment

BhaaLseN Sep 4, 2023

Choose a reason for hiding this comment

JosJuice Sep 4, 2023

Choose a reason for hiding this comment

BhaaLseN Sep 4, 2023

Choose a reason for hiding this comment

twitnic commented Oct 18, 2023

Lobsterzelda commented Oct 22, 2023

TryTwo commented Nov 24, 2023

Lobsterzelda commented Dec 2, 2023

InputEvelution commented Dec 3, 2024