Implement plugin system #11320

IntelOrca · 2020-04-17T17:44:30Z

Closes #3166: Plugins Support For Server

src/openrct2/actions/MazePlaceTrackAction.hpp

Gymnasiast

"Things" need to be renamed to "Entities"

Request has been implemented

distribution/openrct2.d.ts

Gymnasiast

Apart from the (now addressed) compilation error, I didn't notice any problems from just running it without any plug-ins.

I think we should just merge it and have people work with it. After all, it's just after the 0.2.6 release, so plenty of time to iron out the creases.

.github/workflows/ci.yml

distribution/scripting.md

src/openrct2/scripting/ScTile.hpp

tupaschoal

Thanks for taking my issues into consideration, I'll assume you went over them all, as they're resolved (except the one to create the issue for mingw, which I'm guessing you'll file after merging)

distribution/scripting.md

brenoguim

I have yet little knowledge on the code base so I just passed my eyes fishing for C++ patterns I can recognize.

One of the things I didn't mark but I would review is the passing of std::shared_ptr by copy on parameters. They are cheap but involve incrementing/decrementing reference counters which can be a paper cut :)

src/openrct2/scripting/HookEngine.cpp

src/openrct2/scripting/HookEngine.h

src/openrct2/scripting/ScContext.hpp

src/openrct2/scripting/ScDisposable.hpp

src/openrct2/scripting/ScNetwork.hpp

src/openrct2/scripting/ScriptEngine.cpp

src/openrct2/scripting/ScriptEngine.h

janisozaur

I tried reading the code, at least openrct2 part of it, but i'm certain there were spots i missed and I can't claim I understand all of it. The gist of it looks good, though I have some requests.

distribution/scripting.md

janisozaur · 2020-04-23T19:25:36Z

distribution/scripting.md

+
+> Is there a good place to distribute my script to other players?
+
+There is currently no official database for this. For now the recommendation is to upload releases of your script on GitHub alongside your source code (if public). Some people like to make a GitHub repository that just consists of a list of content (scripts in this case) which anyone can add to via pull requests.


There is nothing about versioning of the plugin wrt OpenRCT2 itself. While we can recommend people implement checks for OpenRCT2 tagged version, perhaps we can enforce it in the plugin manifest? OpenRCT2 version would be a rather coarse-grained check, and I'm not fond of adding another one next to network version, so we could use that instead? For networkless builds this is currently set to something like "no multiplayer", but could be easily exposed as well.

There is a minimum API version you can specify on the plugin which corresponds to another number we can change in the code base. Is this what you meant?

src/openrct2-ui/interface/InGameConsole.h

src/openrct2-ui/interface/Widget.cpp

janisozaur · 2020-04-23T20:29:39Z

src/openrct2/scripting/Duktape.hpp

+
+        DukObject(const DukObject&) = delete;
+
+        DukObject(DukObject&& m) noexcept


Shouldn't that imply presence of operator=(DukObject&& other)?

janisozaur · 2020-04-23T20:30:37Z

src/openrct2/scripting/ScriptEngine.h

+    public:
+        DukContext();
+        DukContext(DukContext&) = delete;
+        DukContext(DukContext&& src) noexcept


move assignment too?

Not sure what you mean. This is a move assignment constructor?

src/openrct2/scripting/Plugin.h

src/openrct2-ui/scripting/ScUi.hpp

src/openrct2/scripting/Plugin.h

janisozaur · 2020-04-24T19:03:27Z

I suppose I should've been more specific: does any of the getters mutate its object?

IntelOrca · 2020-04-24T19:31:24Z

@janisozaur A lot of them won't but can't say for certain.

janisozaur · 2020-04-24T19:49:07Z

Err on the const side then.

janisozaur

Dukglue and duktape need to be properly documented in file with the licenses of used projects.

Also dukglue needs documentation on where it was obtained and in which version

Co-Authored-By: Breno Rodrigues Guimarães <brenorg@gmail.com>

This is really unfortunate, but appears to be unavoidable.

Co-Authored-By: Michał Janiszewski <janisozaur@users.noreply.github.com> Co-Authored-By: Aaron van Geffen <aaron@aaronweb.net> Co-Authored-By: Tulio Leao <tupaschoal@gmail.com>

Co-Authored-By: Breno Rodrigues Guimarães <brenorg@gmail.com>

AaronVanGeffen · 2020-04-26T13:48:06Z

We should probably create the 'plugin' folder in the config folder if it doesn't exist yet. At least I had to manually create one (and initially used 'plugins' rather than 'plugin'…)

janisozaur

I still have some requests:

Tests
Uploading plugins tobacktrace
Replay+plugin

and while I'd love to have this issues addressed in the pull request introducing those features, I realise how much effort must've went into preparing current state and fixing those means another(?) indefinite postponing of long awaited feature that seems to work. Let's not forget to file issues for the topics mentioned to be fixed later on.

janisozaur · 2020-04-26T22:08:15Z

ping @pkubaj @PureTryOut @joepie91 @geistesk @ibara this adds new dependency: duktape.

Gymnasiast reviewed Apr 17, 2020

View reviewed changes

src/openrct2/actions/MazePlaceTrackAction.hpp Outdated Show resolved Hide resolved

tupaschoal modified the milestone: v0.2.7 Consideration Apr 19, 2020

Gymnasiast previously requested changes Apr 20, 2020

View reviewed changes

IntelOrca force-pushed the epic/plug-in-4 branch 5 times, most recently from 17ae39a to 0651c20 Compare April 21, 2020 18:38

Gymnasiast reviewed Apr 21, 2020

View reviewed changes

distribution/openrct2.d.ts Show resolved Hide resolved

Gymnasiast approved these changes Apr 21, 2020

View reviewed changes

tupaschoal reviewed Apr 22, 2020

View reviewed changes

IntelOrca force-pushed the epic/plug-in-4 branch 2 times, most recently from 5c05ed4 to 4299de8 Compare April 22, 2020 15:13

janisozaur self-requested a review April 22, 2020 17:34

IntelOrca force-pushed the epic/plug-in-4 branch 2 times, most recently from 3135a52 to 5803cc0 Compare April 22, 2020 22:27

tupaschoal approved these changes Apr 23, 2020

View reviewed changes

brenoguim reviewed Apr 23, 2020

View reviewed changes

distribution/scripting.md Outdated Show resolved Hide resolved

brenoguim reviewed Apr 23, 2020

View reviewed changes

IntelOrca force-pushed the epic/plug-in-4 branch 2 times, most recently from 6c21ced to 4ad4caa Compare April 23, 2020 18:45

janisozaur requested changes Apr 23, 2020

View reviewed changes

janisozaur mentioned this pull request Apr 24, 2020

Support hot-reload of objects #11439

Open

janisozaur reviewed Apr 25, 2020

View reviewed changes

janisozaur force-pushed the epic/plug-in-4 branch from 411d5ce to fa32ca3 Compare April 25, 2020 20:12

IntelOrca and others added 16 commits April 26, 2020 14:35

Apply suggestions from code review

c1ab8b0

Co-Authored-By: Breno Rodrigues Guimarães <brenorg@gmail.com>

Xcode: add FileWatcher, scripting/

3c28861

Disable FileWatcher for macOS

127dd15

Xcode: change deployment target to 10.14.

1619852

This is really unfortunate, but appears to be unavoidable.

Apply suggestions from code review

3fe49ca

Co-Authored-By: Michał Janiszewski <janisozaur@users.noreply.github.com> Co-Authored-By: Aaron van Geffen <aaron@aaronweb.net> Co-Authored-By: Tulio Leao <tupaschoal@gmail.com>

Update src/openrct2/scripting/ScriptEngine.h

4e1fa6a

Co-Authored-By: Breno Rodrigues Guimarães <brenorg@gmail.com>

Update src/openrct2/scripting/ScriptEngine.cpp

ad75cb0

Co-Authored-By: Breno Rodrigues Guimarães <brenorg@gmail.com>

Remove unused getWindow parameter

4162ad0

Revert change to RemoveCustomGameActions

4ebd8da

Rename __ENABLE_SCRIPTING__ to ENABLE_SCRIPTING

c679625

Add minimum API version information to scripting readme

b553850

Add const to ticksElapsed_get

a6eeb8c

Add more const

2c4cf49

Improve dukglue's includes

a0877d1

Try using int32_t instead of size_t

2d43ac5

Fix FileWatcher header file

51ed760

IntelOrca force-pushed the epic/plug-in-4 branch from 5e190d5 to 51ed760 Compare April 26, 2020 13:35

Fix tile type literals

7041b07

IntelOrca added 4 commits April 26, 2020 14:49

Add dukglue and duktape copyright information

90f3a9b

Create plugin directory automatically

c0aa5bd

Implement onClose event for custom windows

ca98ffc

Increment network version

3c02861

janisozaur approved these changes Apr 26, 2020

View reviewed changes

IntelOrca merged commit f9909e7 into OpenRCT2:develop Apr 26, 2020

tupaschoal added this to the v0.2.7 milestone Apr 26, 2020

IntelOrca mentioned this pull request Apr 27, 2020

New project using duktape: OpenRCT2 svaarala/duktape#2289

Closed

AaronVanGeffen mentioned this pull request Jul 15, 2020

Compile errors in older develop builds (v0.2.6). #12277

Closed

Bonsly24 mentioned this pull request Apr 13, 2021

Request to update to most recent version of OpenRCT2 rsn8887/OpenRCT2#4

Open

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Implement plugin system #11320

Implement plugin system #11320

IntelOrca commented Apr 17, 2020

Gymnasiast left a comment •

edited

Gymnasiast left a comment

tupaschoal left a comment

brenoguim left a comment

janisozaur left a comment

janisozaur Apr 23, 2020

IntelOrca Apr 24, 2020

janisozaur Apr 23, 2020

janisozaur Apr 23, 2020

IntelOrca Apr 24, 2020

janisozaur commented Apr 24, 2020

IntelOrca commented Apr 24, 2020

janisozaur commented Apr 24, 2020

janisozaur left a comment

AaronVanGeffen commented Apr 26, 2020

janisozaur left a comment •

edited

janisozaur commented Apr 26, 2020


		> Is there a good place to distribute my script to other players?

		There is currently no official database for this. For now the recommendation is to upload releases of your script on GitHub alongside your source code (if public). Some people like to make a GitHub repository that just consists of a list of content (scripts in this case) which anyone can add to via pull requests.


		DukObject(const DukObject&) = delete;

		DukObject(DukObject&& m) noexcept

Implement plugin system #11320

Implement plugin system #11320

Conversation

IntelOrca commented Apr 17, 2020

Gymnasiast left a comment • edited

Choose a reason for hiding this comment

Gymnasiast left a comment

Choose a reason for hiding this comment

tupaschoal left a comment

Choose a reason for hiding this comment

brenoguim left a comment

Choose a reason for hiding this comment

janisozaur left a comment

Choose a reason for hiding this comment

janisozaur Apr 23, 2020

Choose a reason for hiding this comment

IntelOrca Apr 24, 2020

Choose a reason for hiding this comment

janisozaur Apr 23, 2020

Choose a reason for hiding this comment

janisozaur Apr 23, 2020

Choose a reason for hiding this comment

IntelOrca Apr 24, 2020

Choose a reason for hiding this comment

janisozaur commented Apr 24, 2020

IntelOrca commented Apr 24, 2020

janisozaur commented Apr 24, 2020

janisozaur left a comment

Choose a reason for hiding this comment

AaronVanGeffen commented Apr 26, 2020

janisozaur left a comment • edited

Choose a reason for hiding this comment

janisozaur commented Apr 26, 2020

Gymnasiast left a comment •

edited

janisozaur left a comment •

edited