Update virtual desktop Wine plugin (use Bean) #1073

Zemogiter · 2019-07-09T09:51:04Z

Description

Part of the effort to get rid of direct java calls in JS.
Fixes PhoenicisOrg/phoenicis#2021 that affects every script that needs a virtual desktop. ~~I can't test until PhoenicisOrg/phoenicis#2023 and PhoenicisOrg/phoenicis#2040 are merged~~
I've tested it on Subnautica script and the new plugin works fine.

What works

Everything

What was not tested

Putting screen width and height inside the script that uses the virtual desktop plugin.

Test

Operating system (and linux kernel version): Ubuntu 19.04 5.0.0-20-generic
Hardware (GPU/CPU): i7-7700K and GTX 1080 ti

Ready for review

Script tested as a regular phoenicis user and working (if you have a problem -> create as draft and ask for help).
json-align and eslint run according to the documentation.
Codacy and travis checked.

Engines/Wine/Plugins/virtual desktop/script.js

Using ifs

Moving calls to `screenManager` Bean to separate file.

Adding the header file with calls to `screenManager` Bean.

Removing extra empty lines.

Fixed indentation.

Engines/Wine/Plugins/virtual desktop/script.js

Utils/Functions/System/virtual desktop/script.js

Engines/Wine/Plugins/virtual desktop/script.js

Added missing include to fix Codacy errors and, code cleanup, and removing const

Zemogiter · 2019-07-11T21:55:44Z

Why do I get the "not defined" codacy errors?

Zemogiter · 2019-07-29T15:41:11Z

Upon launching Subnautica script I get this error:

org.graalvm.polyglot.PolyglotException: org.graalvm.polyglot.PolyglotException: No bean named 'ScreenManager' available
	at org.phoenicis.scripts.engine.injectors.EngineInjector.throwException(EngineInjector.java:22)
	at org.phoenicis.scripts.engine.implementation.PolyglotScriptEngine.handleError(PolyglotScriptEngine.java:96)
	at org.phoenicis.scripts.engine.implementation.PolyglotScriptEngine.evalAndReturn(PolyglotScriptEngine.java:75)
	at org.phoenicis.scripts.engine.injectors.IncludeInjector.lambda$injectInto$0(IncludeInjector.java:34)
	at <js> :program(Unnamed:6:206-252)
	at org.graalvm.sdk/org.graalvm.polyglot.Context.eval(Context.java:367)
	at org.phoenicis.scripts.engine.implementation.PolyglotScriptEngine.evalAndReturn(PolyglotScriptEngine.java:73)
	at org.phoenicis.scripts.session.PhoenicisInteractiveScriptSession.eval(PhoenicisInteractiveScriptSession.java:35)
	at org.phoenicis.scripts.interpreter.BackgroundScriptInterpreter.lambda$createInteractiveSession$1(BackgroundScriptInterpreter.java:45)
	at java.base/java.util.concurrent.ThreadPoolExecutor.runWorker(ThreadPoolExecutor.java:1128)
	at java.base/java.util.concurrent.ThreadPoolExecutor$Worker.run(ThreadPoolExecutor.java:628)
	at java.base/java.lang.Thread.run(Thread.java:834)
Caused by host exception: org.phoenicis.scripts.interpreter.ScriptException: org.graalvm.polyglot.PolyglotException: org.graalvm.polyglot.PolyglotException: No bean named 'ScreenManager' available

madoar

To fix the codacy errors you need to add

// eslint-disable-next-line no-unused-vars

before the functions, for an example look at https://github.com/PhoenicisOrg/scripts/blob/master/Utils/Functions/Filesystem/Files/script.js

Utils/Functions/System/virtual desktop/script.js

Zemogiter · 2019-07-29T18:43:26Z

After that change the script starts but hangs after DXVK installation and this error is in the terminal log:

[ERROR] org.phoenicis.multithreading.ControlledThreadPoolExecutorService (l.64) - java.lang.IllegalAccessException: access to public member failed: sun.awt.SunToolkit.getScreenSize[Ljava.lang.Object;@6e8d1337/invokeVirtual, from public Lookup
	at org.graalvm.truffle/com.oracle.truffle.polyglot.HostMethodDesc$SingleMethod$MethodMHImpl.makeMethodHandle(HostMethodDesc.java:318)
	at org.graalvm.truffle/com.oracle.truffle.polyglot.HostMethodDesc$SingleMethod$MHBase.invoke(HostMethodDesc.java:258)
	at org.graalvm.truffle/com.oracle.truffle.polyglot.HostExecuteNode$1.executeImpl(HostExecuteNode.java:776)
	at org.graalvm.truffle/com.oracle.truffle.polyglot.GuestToHostRootNode.execute(GuestToHostRootNode.java:87)
	at <js> :anonymous(Unnamed:28:1009-1060)
	at <js> go(engines.wine.quick_script.steam_script:145:7919-7953)
	at org.graalvm.truffle/com.oracle.truffle.polyglot.ObjectProxyHandler.invoke(HostInteropReflect.java:678)
	at com.sun.proxy.$Proxy47.go(Unknown Source)
	at org.phoenicis.javafx.components.application.skin.ApplicationInformationPanelSkin.lambda$installScript$7(ApplicationInformationPanelSkin.java:237)
	at org.phoenicis.scripts.session.PhoenicisInteractiveScriptSession.eval(PhoenicisInteractiveScriptSession.java:35)
	at org.phoenicis.scripts.interpreter.BackgroundScriptInterpreter.lambda$createInteractiveSession$1(BackgroundScriptInterpreter.java:45)
	at java.base/java.util.concurrent.ThreadPoolExecutor.runWorker(ThreadPoolExecutor.java:1128)
	at java.base/java.util.concurrent.ThreadPoolExecutor$Worker.run(ThreadPoolExecutor.java:628)
	at java.base/java.lang.Thread.run(Thread.java:834)
Caused by host exception: java.lang.IllegalStateException: java.lang.IllegalAccessException: access to public member failed: sun.awt.SunToolkit.getScreenSize[Ljava.lang.Object;@6e8d1337/invokeVirtual, from public Lookup

Exception in thread "pool-3-thread-11" java.lang.IllegalAccessException: access to public member failed: sun.awt.SunToolkit.getScreenSize[Ljava.lang.Object;@6e8d1337/invokeVirtual, from public Lookup
	at org.graalvm.truffle/com.oracle.truffle.polyglot.HostMethodDesc$SingleMethod$MethodMHImpl.makeMethodHandle(HostMethodDesc.java:318)
	at org.graalvm.truffle/com.oracle.truffle.polyglot.HostMethodDesc$SingleMethod$MHBase.invoke(HostMethodDesc.java:258)
	at org.graalvm.truffle/com.oracle.truffle.polyglot.HostExecuteNode$1.executeImpl(HostExecuteNode.java:776)
	at org.graalvm.truffle/com.oracle.truffle.polyglot.GuestToHostRootNode.execute(GuestToHostRootNode.java:87)
	at <js> :anonymous(Unnamed:28:1009-1060)
	at <js> go(engines.wine.quick_script.steam_script:145:7919-7953)
	at org.graalvm.truffle/com.oracle.truffle.polyglot.ObjectProxyHandler.invoke(HostInteropReflect.java:678)
	at com.sun.proxy.$Proxy47.go(Unknown Source)
	at org.phoenicis.javafx.components.application.skin.ApplicationInformationPanelSkin.lambda$installScript$7(ApplicationInformationPanelSkin.java:237)
	at org.phoenicis.scripts.session.PhoenicisInteractiveScriptSession.eval(PhoenicisInteractiveScriptSession.java:35)
	at org.phoenicis.scripts.interpreter.BackgroundScriptInterpreter.lambda$createInteractiveSession$1(BackgroundScriptInterpreter.java:45)
	at java.base/java.util.concurrent.ThreadPoolExecutor.runWorker(ThreadPoolExecutor.java:1128)
	at java.base/java.util.concurrent.ThreadPoolExecutor$Worker.run(ThreadPoolExecutor.java:628)
	at java.base/java.lang.Thread.run(Thread.java:834)
Caused by host exception: java.lang.IllegalStateException: java.lang.IllegalAccessException: access to public member failed: sun.awt.SunToolkit.getScreenSize[Ljava.lang.Object;@6e8d1337/invokeVirtual, from public Lookup

madoar · 2019-07-30T08:15:40Z

The error seems to be unrelated to the changes in this PR. It seems like you're not using the newly created script with the new function Wine.prototype.setVirtualDesktop.

Zemogiter · 2019-07-30T11:44:57Z

~~But I do have the latest version of virtual desktop plugin in my local repo and I keep refreshing repos.~~
Sorry did not understood. I thought you meant the virtual desktop plugin. Now I know you meant the game script. Basically we need to change this in every script that uses the plugin:

remove this line:

scripts/Applications/Games/Subnautica/Steam/script.js

Line 24 in 749c609

var screenSize = java.awt.Toolkit.getDefaultToolkit().getScreenSize();
change this line:

scripts/Applications/Games/Subnautica/Steam/script.js

Line 25 in 749c609

wine.setVirtualDesktop(screenSize.width, screenSize.height);

to this:

wine.setVirtualDesktop();

Can we do this automatically?

plata · 2019-07-31T17:29:01Z

Should be possible with search and replace but actually I do not think that we have many scripts using this.

Zemogiter · 2019-07-31T19:27:22Z

We have 13 scripts in total (based on runing grep -r "virtual_desktop" "./" in local repo):

Tom Clancy's Rainbow Six 3 : Raven Shield (Local)
Tom Clancy's Rainbow Six 3 : Raven Shield (Steam)
Caesar III (Local)
Caesar III (Steam)
Anno 2070 (Local)
Anno 2070 (Uplay)
Guild Wars 2 (Online)
Guild Wars 2 (Local)
Subnautica
PC Building Simulator
Wildlife Park 2 (Local)
Wildlife Park 2 (Steam)
Subnautica Below Zero

madoar · 2019-08-03T11:23:17Z

@Zemogiter what editor are you using to work on the scripts? Are you using a "simple" editor like gedit, or are you using something more sophisticated like Atom or Visual Studio Code?

I would recommend you to use something more sophisticated when working with the scripts. For example I currently use Visual Studio Code to work with the scripts. VS Code comes with a search and replace function that works on a folder hierarchy.

Trying placing the eslint-disable line at the top to get rid of Codacy error.

Engines/Wine/Plugins/virtual desktop/script.js

madoar

I would like to block this until #1076 is merged

Engines/Wine/Plugins/virtual desktop/script.js

Added an include in order to be compatible with PhoenicisOrg#1076

Engines/Wine/Plugins/virtual desktop/script.js

Utils/Functions/System/virtual desktop/script.js

Moving includes around.

Zemogiter · 2019-08-11T19:03:00Z

Ready for review.

Restored the definitions.

madoar

Is this the only place where we accessed the screen width and height from inside the scripts?
If no when do you want to change the other occurrences?

I suggest to do all related changes at once in this case. This makes it easier for us to review this and it is also makes this easier to test

Engines/Wine/Plugins/virtual desktop/script.js

madoar · 2019-08-15T21:17:50Z

@plata I think Codacy is broken... I think it should show at least two issues (for the undefined usage of getScreenWidth and getScreenHeight) but it does not...

Updated includes

Zemogiter · 2019-08-16T09:22:40Z

Is this the only place where we accessed the screen width and height from inside the scripts?
If no when do you want to change the other occurrences?

I suggest to do all related changes at once in this case. This makes it easier for us to review this and it is also makes this easier to test

I don't see anywhere else in the repo where we access screen dimensions.

madoar · 2019-08-17T09:17:21Z

@Zemogiter I mean: when do you want to do the changes you suggested in #1073 (comment) and
#1073 (comment)?

plata · 2019-08-17T10:55:37Z

@madoar does ESLint show an issue if you run it manually?

Zemogiter · 2019-08-17T17:38:53Z

@madoar preferably right after this PR is merged but do I have to make a separate PR for each script?

madoar · 2019-08-17T17:41:27Z

@plata I've tried eslint for this PR, but I've seen the issue for the later commits of #1076

@Zemogiter No you don't need to open a new PR for each script it is ok (and I think in this case also better) to open a single PR where you update all scripts.

madoar

@plata if you have no change requests I think we can merge this PR.

madoar · 2019-08-18T10:28:32Z

@ImperatorS79 you also need your approval before we can merge :)

- adds a new script to access the "screenManager" Java Bean

Update script.js

abc2d55

madoar requested changes Jul 9, 2019

View reviewed changes

Engines/Wine/Plugins/virtual desktop/script.js Outdated Show resolved Hide resolved

Engines/Wine/Plugins/virtual desktop/script.js Outdated Show resolved Hide resolved

Update script.js

5550ad0

plata changed the title ~~Update virtual desktop, using beans~~ Update virtual desktop Wine plugin (use Bean) Jul 10, 2019

ImperatorS79 requested changes Jul 10, 2019

View reviewed changes

Engines/Wine/Plugins/virtual desktop/script.js Outdated Show resolved Hide resolved

Zemogiter added 5 commits July 10, 2019 20:35

Update script.js

234ebf0

Using ifs

Update script.js

a8c2b94

Moving calls to `screenManager` Bean to separate file.

Add files via upload

2be9384

Adding the header file with calls to `screenManager` Bean.

Update application.json

c31e67f

Removing extra empty lines.

Update script.js

3781c3e

Fixed indentation.

madoar requested changes Jul 11, 2019

View reviewed changes

Zemogiter added 2 commits July 11, 2019 23:22

Update script.js

34e2cbb

Added missing include to fix Codacy errors and, code cleanup, and removing const

Update script.js

66e6a80

Zemogiter added 4 commits July 12, 2019 08:47

Update script.js

97716b0

Update script.js

1169d54

Update script.js

1d3fd5b

Update script.js

903fcf8

madoar requested changes Jul 29, 2019

View reviewed changes

Utils/Functions/System/virtual desktop/script.js Outdated Show resolved Hide resolved

Zemogiter added 2 commits July 29, 2019 20:37

Update script.js

f20ae8b

Update script.js

928e664

Update script.js

2f7fbdf

Zemogiter mentioned this pull request Jul 31, 2019

PC Building Simulator script crashes PhoenicisOrg/phoenicis#2021

Closed

Update script.js

e82fced

Trying placing the eslint-disable line at the top to get rid of Codacy error.

madoar reviewed Aug 4, 2019

View reviewed changes

Engines/Wine/Plugins/virtual desktop/script.js Outdated Show resolved Hide resolved

Update script.js

5748640

madoar reviewed Aug 4, 2019

View reviewed changes

Engines/Wine/Plugins/virtual desktop/script.js Outdated Show resolved Hide resolved

Zemogiter added 2 commits August 4, 2019 16:57

Update script.js

73d2f81

Update script.js

7271bfe

madoar requested changes Aug 10, 2019

View reviewed changes

Engines/Wine/Plugins/virtual desktop/script.js Show resolved Hide resolved

Zemogiter added 3 commits August 11, 2019 11:48

Update script.js

db69831

Added an include in order to be compatible with PhoenicisOrg#1076

Update script.js

755c035

Merge branch 'master' into patch-4

5a819b1

madoar requested changes Aug 11, 2019

View reviewed changes

Zemogiter added 2 commits August 11, 2019 11:57

Update script.js

ef28adc

Moving includes around.

Update script.js

9910f44

Update script.js

4329f07

Restored the definitions.

madoar requested changes Aug 15, 2019

View reviewed changes

Engines/Wine/Plugins/virtual desktop/script.js Outdated Show resolved Hide resolved

Update script.js

171df96

Updated includes

madoar approved these changes Aug 17, 2019

View reviewed changes

plata approved these changes Aug 18, 2019

View reviewed changes

ImperatorS79 approved these changes Aug 18, 2019

View reviewed changes

madoar merged commit a385d1c into PhoenicisOrg:master Aug 18, 2019

Zemogiter deleted the patch-4 branch August 18, 2019 12:54

petermetz pushed a commit to petermetz/scripts that referenced this pull request Jun 7, 2020

Update virtual desktop Wine plugin (use Bean) (PhoenicisOrg#1073)

a24e1bc

- adds a new script to access the "screenManager" Java Bean

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Update virtual desktop Wine plugin (use Bean) #1073

Update virtual desktop Wine plugin (use Bean) #1073

Zemogiter commented Jul 9, 2019 •

edited

Loading

Zemogiter commented Jul 11, 2019

Zemogiter commented Jul 29, 2019

madoar left a comment

Zemogiter commented Jul 29, 2019

madoar commented Jul 30, 2019

Zemogiter commented Jul 30, 2019 •

edited

Loading

plata commented Jul 31, 2019

Zemogiter commented Jul 31, 2019

madoar commented Aug 3, 2019

madoar left a comment

Zemogiter commented Aug 11, 2019

madoar left a comment

madoar commented Aug 15, 2019

Zemogiter commented Aug 16, 2019

madoar commented Aug 17, 2019

plata commented Aug 17, 2019

Zemogiter commented Aug 17, 2019

madoar commented Aug 17, 2019

madoar left a comment

madoar commented Aug 18, 2019

Update virtual desktop Wine plugin (use Bean) #1073

Update virtual desktop Wine plugin (use Bean) #1073

Conversation

Zemogiter commented Jul 9, 2019 • edited Loading

Description

What works

What was not tested

Test

Ready for review

Zemogiter commented Jul 11, 2019

Zemogiter commented Jul 29, 2019

madoar left a comment

Choose a reason for hiding this comment

Zemogiter commented Jul 29, 2019

madoar commented Jul 30, 2019

Zemogiter commented Jul 30, 2019 • edited Loading

plata commented Jul 31, 2019

Zemogiter commented Jul 31, 2019

madoar commented Aug 3, 2019

madoar left a comment

Choose a reason for hiding this comment

Zemogiter commented Aug 11, 2019

madoar left a comment

Choose a reason for hiding this comment

madoar commented Aug 15, 2019

Zemogiter commented Aug 16, 2019

madoar commented Aug 17, 2019

plata commented Aug 17, 2019

Zemogiter commented Aug 17, 2019

madoar commented Aug 17, 2019

madoar left a comment

Choose a reason for hiding this comment

madoar commented Aug 18, 2019

Zemogiter commented Jul 9, 2019 •

edited

Loading

Zemogiter commented Jul 30, 2019 •

edited

Loading