[WIP] Add Python Scripting support #7064

Felk · 2018-06-03T22:45:05Z

Scripting support is already functional, but still being worked on to get to a mergable state. You can try out the most recent state over at my fork at felk/dolphin. See its readme for further instructions.

I am currently in the process of adding scripting support in the form of embedded python scripts, which will work in a simple event-callback-based fashion. A script can basially register callbacks on events at startup, and also call back into functions defined by dolphin, e.g. reading memory etc., similar to how it's done in other emulators like Bizhawk or vba. I also added some async/await magic, but that's just convenience specific to python I like.

A script might look like this:

from dolphin import event, memory

# framecount in Pokemon Battle Revolution
addr = 0x8063fc2c

def onframeadvance():
    value = memory.read_u32(addr)
    print(f"Internal framecount: {value}")

event.onframeadvance(onframeadvance)

Minimal scope goals:

put the python external into a submodule and add a readme on how to collect the required files from scratch for maintenance.
add basic memory reading/writing support
add controller input support
add basic support for on-screen drawing
add savestate support
add frameadvance event
command line flag for automatic start of a script

The scripting API surface shall be implemented as their need arises and this get more feedback.

Polishing goals:

Linux support
MacOS support
add GUI for running scripts.
no global state in python implementation (use of sub-interpreters)
extend API surface to something that would be perceived as servicable and not just "minimal". I don't have a concrete list of what that would contain.

Open for discussion:

Is Python a good choice? Should default scripting support be in LUA or something else instead? If Python is OK, then:
- what about the additional file size? In its current form it would add ~4.7 MB to the zipped file.
- should it be disabled (as in, not compiled) by default?

The decoupling between the scripting implementation and interfacing to the core is going over an API namespace proxying commands to the core and decoupling events. Have a graph of how it roughly looks design-wise here.

lioncash

Left some comments.

I'm very not fond of the way many of the modules are file-scope based and not class-based. This means program execution has to wait until all of those objects are constructed before anything else can be done, which is pretty wasteful if a person has no intention of even using scripting. If someone doesn't use scripting, they shouldn't have resource overhead just from it merely existing.

Source/Core/Core/PowerPC/MMU.cpp

+
+s8  HostRead_S8 (const u32 address)
+{
+  return Common::BitCast<s8>(HostRead<u8> (address));


Source/Core/Core/PowerPC/MMU.cpp

@@ -555,24 +577,50 @@ void Write_F64(const double var, const u32 address)
  Write_U64(integral, address);
 }

-u8 HostRead_U8(const u32 address)
+template<typename T>
+T HostRead(const u32 address)


Source/Core/Core/PowerPC/MMU.cpp

+
+void HostWrite_S8 (const s8  var, const u32 address)
+{
+  HostWrite<u8> (Common::BitCast<u8>(var), address);


Source/Core/Core/PowerPC/MMU.cpp

@@ -589,24 +637,47 @@ double HostRead_F64(const u32 address)
  return Common::BitCast<double>(integral);
 }

-void HostWrite_U8(const u8 var, const u32 address)
+template<typename T>
+void HostWrite(const T var, const u32 address)


Source/Core/Core/Scripting/Python/PyScriptingBackend.h

+void Init();
+void Shutdown();
+
+typedef void (*Task)();


Source/Core/Core/Scripting/Python/Pyr.h

+bool ParseTupleSimple(PyObject* args, T1* arg1, T2* arg2)
+{
+  const std::string format =
+      std::string(Pyr::get_pyformat<T1>()) + std::string(Pyr::get_pyformat<T2>());


Source/Core/Core/Scripting/Python/PyScriptingBackend.h

@@ -0,0 +1,12 @@
+#pragma once


Source/Core/Core/Scripting/Python/PyScriptingBackend.cpp

+    UTF8ToUTF16(File::GetExeDirectory()) + L"/python36-embed;" +
+    UTF8ToUTF16(File::GetExeDirectory());  // TODO can this be done more elegantly?
+
+static std::deque<Task> tasks;


Source/Core/Core/Scripting/Python/PyScriptingBackend.cpp

+    UTF8ToUTF16(File::GetExeDirectory());  // TODO can this be done more elegantly?
+
+static std::deque<Task> tasks;
+static CoreTiming::EventType* scripting_event = nullptr;


Source/Core/Core/Scripting/Python/aeventmodule.h

@@ -0,0 +1,57 @@
+#pragma once


lioncash

Realized I didn't comment on Event.h/Event.cpp, whoops.

Source/Core/Core/API/Events.cpp

@@ -0,0 +1,23 @@
+
+#include "Events.h"


Source/Core/Core/API/Events.h

+
+// a listener on T is any function that takes a const reference to T as argument
+template <typename T>
+using Listener = void (*)(const T&);


Source/Core/Core/API/Events.h

+template <typename T>
+void EmitEvent(T evt)
+{
+  for (Listener<T> listener : GetListenersVector<T>())


Source/Core/Core/API/Events.h

+template <typename T>
+void ListenEvent(Listener<T> listener)
+{
+  GetListenersVector<T>().push_back(listener);


Warepire · 2018-06-04T07:57:44Z

For feedback on your question regarding Python vs Lua by users, one user group will be the people at TASVideos. Currently as you saw most emulators use Lua, but I do not think Python is completely out of the picture (personally I'm a bigger fan of Python). Pop by their IRC or start a discussion on the forums to gather data.

Felk · 2018-06-04T20:19:03Z

@lioncash

I'm very not fond of the way many of the modules are file-scope based and not class-based

Regarding the python module functions, they unfortunately require me to declare them as typedef PyObject *(*PyCFunction)(PyObject *, PyObject *);. For all the other stuff, I could put it in classes, but those will effectively be singletons (or at least not work with multiple instances), if that's what you mean.

lioncash

@Felk re: functions in a list std::function instances have a .target() function which retrieves the address of the stored callable object. It may be worthwhile adding a better means of identifying the functions however (it wouldn't be wise to rely on anonymous function/lambda addresses).

For all the other stuff, I could put it in classes, but those will effectively be singletons (or at least not work with multiple instances), if that's what you mean.

Note that this throws a massive wrench in the plans I was slowly working towards over time which was eliminating most mutable global state (yes, even if it's static and file scope, it's still considered a global as far as the language cares) in order to allow multiple instances of the core to be run in the same process, which would be preferable for making the core a proper library in the future. It's also worth noting that one of the requirements I urged should be met for the implementation was "no global state" (where avoidable), and on closer inspection of the current implementation I see this is... really kind of not the case. Scripting is inherently driven from the frontend for the core, so why is it not situated around the frontends initializing the scripting? (currently it's grafted into Core.cpp). I remember @delroth bringing up ideas related to scripting that would also be usable UI-side (I can't remember off the top of my head what they were though).

Again, a frontend shouldn't have overhead for something that isn't being used at the time by someone, and when something is finished being used, those resources shouldn't stick around, currently this operates orthogonal to that.

Aside from that, why are most of the modules implemented as headers as opposed to having a paired .cpp file with it? Many of these have static variables in them, which can cause odd issues related to addresses (as every translation unit gets their own instance of those static variables). So an accidental usage of something in a source file can lead to difficult to debug situations. It also means any change to implementation code requires rebuilding everything that includes it. Not a big con, but it still exists.

Convention-wise:

All private class variables should have an m_ prefix, and file-static variables should have s_ on them. Globals with external linkage should have a g_ on them, but I really hope that doesn't become necessary.
Public interface should come before private interface. Public interface is what people are going to be reading and using primarily. It's not useful to put things devs shouldn't care about 99% of the time as the first thing in a class body.
All functions in the codebase should follow, ThisIsAName() (where initialisms are upper-cased, for example, GetID() instead of GetId()). This doesn't mean the actual scripting API has to be like that however. Conventions for that haven't even been discussed, I think (which probably should be at some point, if that's the case, and documented somewhere).

Source/Core/Core/Scripting/Python/aeventmodule.h

+  char* x;
+  if (!PyrArg::ParseTupleSimple(args, &x))
+    return nullptr;
+  if (std::string(x) == "frameadvance")


Source/Core/Core/Scripting/Python/aeventmodule.h

+
+namespace PyScripting
+{
+inline const char* pycode = R"(


Source/Core/Core/Scripting/Python/Pyr.h

+  {
+    return PyBool_FromLong(n);
+  }
+  else if constexpr (std::is_floating_point<T>::value)


Source/Core/Core/Scripting/Python/Pyr.h

+  {
+    return PyFloat_FromDouble(n);
+  }
+  else if constexpr (std::is_integral<T>::value)


Source/Core/Core/Scripting/Python/Pyr.h

+  else if constexpr (std::is_integral<T>::value)
+  {
+    static_assert(sizeof(T) <= sizeof(unsigned long long), "integral type too big, not supported");
+    const bool is_signed = std::is_signed<T>::value;


Source/Core/Core/Scripting/Python/Pyr.h

+template <typename T>
+constexpr const char* GetPyFormat()
+{
+  if constexpr (std::is_same<T, signed char>::value)


Source/Core/Core/Core.vcxproj

@@ -1,595 +1,616 @@
-<?xml version="1.0" encoding="utf-8"?>


Source/Core/Core/Core.vcxproj.filters

@@ -1,1587 +1,1622 @@
-<?xml version="1.0" encoding="utf-8"?>


Felk · 2018-06-05T18:44:09Z

@lioncash

regarding global state: Python can run multiple "sub-interpreters", so it's definetely possible to get it to work in class scope, with some more effort. The exception to this still are the python extension functions though. Your idea of getting it out of the core sounds even better though, as I don't plan on having the rest of the core be coupled to the scripting anyway, and instead go over that API-namespace. That part can probably be put into classes without trouble. I can try doing that for this PR already if you want.
If the scripting is in its own little project, would you still want it to allow for multiple instances? Or would that be okay roughly the way it is now?

regarding header-only files: That's just me being lazy with the still prototype-y code. I'll clean that up.

regarding all the code reviews: I'll try to put code I consider ready for review into dedicated commits, so all the WIP-code is situated in the most recent commit and you don't have to look at code I already know I will change soon

lioncash · 2018-06-06T01:29:24Z

I can try doing that for this PR already if you want.

That would actually be fantastic.

If the scripting is in its own little project, would you still want it to allow for multiple instances?

This would make it considerably more flexible in terms of ways it can be used (and in terms of how the rest of the codebase can change around it in lifecycle-related means).

Felk · 2018-07-18T20:10:39Z

Sorry I have not worked much on this recently. I plan on getting a minimal solution done which basically contains scripting support for memory editing, input, basic on-screen drawing, and some events. Because I have never done a lot of these things, I don't want to be too ambitious and risk my little project here completely perishing. For that reason, I postponed the following things:

the python scripting project will just have static methods for now. To fully avoid global state I'll need to dive deeper into how subinterpreters work eventually.
No UI for selecting scripts. Not gonna open a whole new can of worms, that is QT. Instead, I'll just add a command line parameter for now.
I will collect the functionality offered to scripting in the API namespace I already created, but make no attempts trying to finetune it for anything else. This means disregarding the planned "universal dolphin API" thing. I do want it to be a good start to build off of though if someone does put effort into such an API eventually.
Support for not-Windows. This is just a matter of either finally diving into CMake, or getting help from someone to recreate the build steps I added for msbuild

Even if after that point it's not ready for a merge (it won't be), I still want to "publish" it to a few people (annoy a few tas'ers, dolphin devs, tpp devs, and myself; reimplementing that pbrengine thing is still a goal of mine) to get early feedback and refine the minimal scope. Polishing (or scrapping the whole project, who knows) comes after.

I edited the original post accordingly to track these goals.

Felk · 2019-08-16T19:25:01Z

sorry, this is dead and it's unlikely I'll ever pick it up again and finish

Felk · 2020-08-04T19:28:38Z

Assuming that I did continue to work on this branch, and got it to a working state that also addressed some of the original issues (like no global state), would you want to take another look, even if there is no intent (in the short term at least) to actually merge it?

Basically I'm asking for permission to waste your time looking over it and to re-open this as WIP with the possibility of it staying a fork anyway.

Felk · 2020-11-20T18:00:25Z

As well as this, the code contained for the scripting API uses ImGUI, but the code is not well separated from Core and as such ImGUI now needs to be linked to the Core section of code in order for it to build (which isn't ideal in certain build circumstances eg headless, libretro etc)

@AlexApps99 I tried to understand the issue more deeply and got confused: It seems that Core needs to always link against VideoCommon anways (e.g. for OnScreenDisplay), and VideoCommon already depends on ImGui. Can you please explain a scenario to me that I can reproduce where removing the ImGui dependency from the Core would result in something that does not depend on ImGui in other places?

AlexApps99 · 2020-11-20T19:20:26Z

That is a good point you make.
I am not very experienced with Dolphin's codebase, so it's possible it's not an issue after all.

I will try to contact a dev to see if they agree.
Thanks for bringing this up

Edit: I talked to a dev yesterday, but they were unsure.
I suppose we could ignore the issue for now and address it later when another dev can evaluate it

add linux support

AlexApps99 · 2021-04-11T00:22:08Z

@Felk do you still have plans to work on this? Totally fine if not, just wondering

Felk · 2021-04-12T12:34:09Z

I do, but unfortunately I usually program for a couple of weeks and then completely lose interest for months or years. That's not deliberate, I haven't abandoned this, my working habits are just very unpredictable.

AlexApps99 · 2021-04-12T19:35:53Z

Ok, that's good to hear :)

This fixes a follow-up of 7a6b63b "MMU: Don't truncate 64-bit values when calling Memcheck()".

…is drawn I believe (haven't tested) that this is different to how e.g BizHawk does it. Their Lua documentation for events mentions `event.onframeend` to happen "after all emulation and drawing has completed" and as the "'default behavior of lua scripts". However, doing it this way instead feels more intuitive for the default behavior, because otherwise everything that happens between frames can only be observed after the frame has just been drawn, requiring one more frame to pass before being able to display anything using e.g. `gui.draw_text`.

Previously there was no check if a button was omitted in the button map, and the emulator just crashed. Now omitting button keys is allowed and will assume to the unpressed/default state.

Zopolis4 · 2022-06-08T10:08:41Z

Readme.md

-in the ISO list and selecting "ISO Properties".
+There is nothing fundamentally stopping this to work on ARM as well,
+but currently only the Python [externals](Externals) for Windows x86-64 are bundled.
+There are no embeddable Python distribution for ARM64 readily available on python.org,


The latest 3.11 prereleases have ARM64 embeddable packages for Windows, so unless 3.11 somehow breaks compatibility that's something to consider when 3.11 gets a stable release.

oh, that's good to know, thanks!

leoetlino added WIP / do not merge Work in progress (do not merge) RFC Request for comments labels Jun 3, 2018

lioncash reviewed Jun 3, 2018

View reviewed changes

Felk force-pushed the scripting branch from 538cf22 to 44b8406 Compare June 4, 2018 22:09

lioncash reviewed Jun 4, 2018

View reviewed changes

spycrab reviewed Jun 5, 2018

View reviewed changes

Felk force-pushed the scripting branch 2 times, most recently from 234941a to 3de7ac3 Compare June 6, 2018 23:11

Felk force-pushed the scripting branch 2 times, most recently from aa20c28 to c6a4394 Compare July 18, 2018 20:43

Felk closed this Aug 16, 2019

Felk added 11 commits August 5, 2020 22:07

always enable address translation for Host* memory functions

aba2010

add external python dependency

7f2d36f

add bare Scripting project

292549e

add basic event system to core

34cb6e6

add signed HostRead/HostWrite functions to MMU

77269ac

hook up ScriptingEngine to Main

6342eda

add basic memory functions to core API

5f4da40

bump Externals/python to 3.8.5

96f338f

API::Events: add functionality to listen to event once

eb03ddd

add --script command line argument

f9e2ed1

hook up basic python scripting backend

ce8c068

Felk added 6 commits November 22, 2020 15:12

Merge pull request #6 from Felk/cmake

7a84328

add linux support

Readme: update notes regarding debugging

000c20b

Readme: remove notes regarding linux support

4b19ef5

Merge remote-tracking branch 'upstream/master' into sync-fork

57cfc9b

fix warnings in ScriptsListModel.cpp

51d5a26

re-apply msbuild changes from !9035 to Scripting

3c39a2a

smurf3tte mentioned this pull request Dec 17, 2020

Debugger: Conditional breakpoints #9334

Closed

README: mention setting log level to Error or lower

99e4c1b

Felk added 16 commits October 21, 2021 18:58

Merge remote-tracking branch 'upstream/master' into sync-fork

aba2e89

Scripting API: make MemoryBreakpoint value u64

d4e2424

This fixes a follow-up of 7a6b63b "MMU: Don't truncate 64-bit values when calling Memcheck()".

use C++20, promote std::experimental::erase_if to std::erase_if

f148a6b

add cmake-build-debug/ to .gitignore, is generated by JetBrains CLion

9da2933

DolphinLib: add missing project reference to Scripting

3c354a0

fix compile issues after merge in MMU.cpp

55099ed

Scripting: fix deadlocks when using savestates

ddcf995

Merge branch 'sync-fork'

be06a5f

reorganized python stub files

7a64a17

Scripting: mark positional-only arguments as such in the stubs

37fb29a

improve Scripting 'Getting Started' section a bit

a700685

use working directory instead of exe directory for script files

60ad1db

move emitting FrameAdvance event to OnFrameEnd, simplifies code

663fbb4

Scripting: allow omitting button keys for controller API

54e791a

Previously there was no check if a button was omitted in the button map, and the emulator just crashed. Now omitting button keys is allowed and will assume to the unpressed/default state.

Scripting: include working directory in module search path

46b7eac

Zopolis4 reviewed Jun 8, 2022

View reviewed changes

[WIP] Add Python Scripting support #7064

Are you sure you want to change the base?

[WIP] Add Python Scripting support #7064

Conversation

Felk commented Jun 3, 2018 • edited Loading

lioncash left a comment

Choose a reason for hiding this comment

This comment was marked as off-topic.

This comment was marked as off-topic.

This comment was marked as off-topic.

This comment was marked as off-topic.

This comment was marked as off-topic.

This comment was marked as off-topic.

This comment was marked as off-topic.

This comment was marked as off-topic.

This comment was marked as off-topic.

This comment was marked as off-topic.

lioncash left a comment

Choose a reason for hiding this comment

This comment was marked as off-topic.

This comment was marked as off-topic.

This comment was marked as off-topic.

This comment was marked as off-topic.

This comment was marked as off-topic.

Warepire commented Jun 4, 2018

Felk commented Jun 4, 2018 • edited Loading

lioncash left a comment

Choose a reason for hiding this comment

This comment was marked as off-topic.

This comment was marked as off-topic.

This comment was marked as off-topic.

This comment was marked as off-topic.

This comment was marked as off-topic.

This comment was marked as off-topic.

This comment was marked as off-topic.

This comment was marked as off-topic.

This comment was marked as off-topic.

Felk commented Jun 5, 2018

lioncash commented Jun 6, 2018

Felk commented Jul 18, 2018

Felk commented Aug 16, 2019

Felk commented Aug 4, 2020

Felk commented Nov 20, 2020

AlexApps99 commented Nov 20, 2020 • edited Loading

AlexApps99 commented Apr 11, 2021

Felk commented Apr 12, 2021

AlexApps99 commented Apr 12, 2021

Zopolis4 Jun 8, 2022 • edited Loading

Choose a reason for hiding this comment

Felk Jun 8, 2022

Choose a reason for hiding this comment

Felk commented Jun 3, 2018 •

edited

Loading

Felk commented Jun 4, 2018 •

edited

Loading

AlexApps99 commented Nov 20, 2020 •

edited

Loading

Zopolis4 Jun 8, 2022 •

edited

Loading