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

Update tahoma and mfc42 verb: use fileExists #1116

Merged
merged 29 commits into from
Nov 28, 2019

Conversation

ImperatorS79
Copy link
Contributor

@ImperatorS79 ImperatorS79 commented Sep 6, 2019

Description

Fix a part of #1107 relating mfc42.

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.

Merge Phoenicis/Scripts:master into ImperatorS79/Scripts:master
updated Hearthstone (download application directly) (PhoenicisOrg#383)
@ImperatorS79
Copy link
Contributor Author

@Zemogiter could you test if this fixes your errors with mfc42 and tahoma ?

@madoar
Copy link
Collaborator

madoar commented Sep 7, 2019

@ImperatorS79 I would like to wait until #1112 has been merged before we merge this

@Zemogiter
Copy link
Contributor

@ImperatorS79 mfc42 works but in Tahoma we need extra ) in

if(!fileExists(this.fontDirectory()) {

@ImperatorS79
Copy link
Contributor Author

@Zemogiter can you test again ?

@@ -19,6 +19,10 @@ class Tahoma {
const wizard = this.wine.wizard();
const prefixDirectory = this.wine.prefixDirectory();
const fontDirectory = this.wine.fontDirectory();

if (!fileExists(fontDirectory)) {
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The method name sounds strange in this case. Should we add a dedicated method for folders? What do you think @plata @ImperatorS79?

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We can do that. Would basically be an alias then, right?

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actually I'm not sure. If we decide to split the functionality into a fileExists and directoryExists method we could also test whether the given path is not a file/directory but instead a directory/file, depending on the regarded method.

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sounds reasonable.

@plata
Copy link
Collaborator

plata commented Sep 11, 2019

Please fix the PR title before merging.

@ImperatorS79 ImperatorS79 changed the title File exists Update tahoma and mfc42 verb: use fileExists Sep 11, 2019
@Zemogiter
Copy link
Contributor

@ImperatorS79 at the begining of .cab file extraction I get the error saying the fonts directory does not exist but it does

Extracting to: /home/jonasz/.Phoenicis/containers//wineprefix//RimWorld//drive_c/tahoma/
/home/jonasz/.Phoenicis/resources///IELPKTH.CAB: WARNING; possible 5592 extra bytes at end of file.
Extracting cabinet: /home/jonasz/.Phoenicis/resources///IELPKTH.CAB

All done, no errors.
[ERROR] org.phoenicis.multithreading.ControlledThreadPoolExecutorService (l.64) - Path "/home/jonasz/.Phoenicis/containers//wineprefix//RimWorld//drive_c/windows/Fonts" does not exist
	at org.phoenicis.tools.files.FileUtilities.copy(FileUtilities.java:139)
	at <js> cp(Unnamed:56:1251-1284)
	at <js> go(Unnamed:40:1231-1295)
	at <js> install(Unnamed:57:1761-1781)
	at org.graalvm.sdk/org.graalvm.polyglot.Value.invokeMember(Value.java:457)
	at org.phoenicis.engines.VerbsManager.lambda$installVerb$0(VerbsManager.java:71)
	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.IllegalArgumentException: Path "/home/jonasz/.Phoenicis/containers//wineprefix//RimWorld//drive_c/windows/Fonts" does not exist

Exception in thread "pool-3-thread-5" Path "/home/jonasz/.Phoenicis/containers//wineprefix//RimWorld//drive_c/windows/Fonts" does not exist
	at org.phoenicis.tools.files.FileUtilities.copy(FileUtilities.java:139)
	at <js> cp(Unnamed:56:1251-1284)
	at <js> go(Unnamed:40:1231-1295)
	at <js> install(Unnamed:57:1761-1781)
	at org.graalvm.sdk/org.graalvm.polyglot.Value.invokeMember(Value.java:457)
	at org.phoenicis.engines.VerbsManager.lambda$installVerb$0(VerbsManager.java:71)
	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.IllegalArgumentException: Path "/home/jonasz/.Phoenicis/containers//wineprefix//RimWorld//drive_c/windows/Fonts" does not exist

@ImperatorS79
Copy link
Contributor Author

ImperatorS79 commented Sep 14, 2019

Yes, I have the same kind of error in other verbs when testing Adobe Photoshop script, that says system32 does not exist. But it was actually that it did not find the file to copy.

I do not know why we have this error.

@madoar
Copy link
Collaborator

madoar commented Sep 14, 2019

The error only occurs when a CabExtract is followed by a cp instruction right?
If this is the case then maybe CabExtract returns before the filesystem has been updated/refreshed?

Can you try adding a java.lang.Thread.sleep(100); instruction between CabExtract and cp?

@Zemogiter
Copy link
Contributor

@madoar done that, still getting the same error

@madoar
Copy link
Collaborator

madoar commented Sep 14, 2019

I'm still confuses from which line in which script this error is thrown... The issue is that the source path/directory in a cp operation does not exist. So which script calls cp(fontDirectory, ...) in line 55? I am unable to even find a script that calls cp(fontDirectory, ...) at all...

@plata
Copy link
Collaborator

plata commented Sep 16, 2019

You can add some log output like

print("1");
// do something
print("2");
// do something more

to figure out where exactly the error is caused.

@madoar
Copy link
Collaborator

madoar commented Oct 5, 2019

Are there any updates on this?

@Zemogiter
Copy link
Contributor

Zemogiter commented Oct 30, 2019

Just copied the moded verbs and getting this error durring Sims 3 installation:

org.graalvm.polyglot.PolyglotException: TypeError: <this>.wine.overrideDLL is not a function
    at <js>.go (Unnamed:48)
    at <js>.:anonymous (Unnamed:20)
    at <js>.go (Unnamed:83)
    at com.oracle.truffle.polyglot.ObjectProxyHandler.invoke (HostInteropReflect.java:678)
    at com.sun.proxy.$Proxy64.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.util.concurrent.ThreadPoolExecutor.runWorker (ThreadPoolExecutor.java:1128)
    at java.util.concurrent.ThreadPoolExecutor$Worker.run (ThreadPoolExecutor.java:628)
    at java.lang.Thread.run (Thread.java:834)

Runing the latest source code and script repo.

@madoar
Copy link
Collaborator

madoar commented Nov 1, 2019

Yes the PR needs to be rebased. The changes to the plugins are not included.

@Zemogiter
Copy link
Contributor

Zemogiter commented Nov 17, 2019

@ImperatorS79 please rebase

Copy link
Collaborator

@madoar madoar left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This PR needs to be rebased

@Zemogiter
Copy link
Contributor

Too bad we can't use dependabot in non-dependabot PRs to rebase them.

@madoar madoar added the requires rebase This PR requires a git rebase label Nov 25, 2019
@plata
Copy link
Collaborator

plata commented Nov 26, 2019

https://github.com/cirrus-actions/rebase

@Zemogiter
Copy link
Contributor

@ImperatorS79 please remove trailing spaces

@plata
Copy link
Collaborator

plata commented Nov 28, 2019

If you're interested in having this rebase action, please open an issue regarding it. Then we can discuss.

@madoar madoar merged commit aab3342 into PhoenicisOrg:master Nov 28, 2019
petermetz pushed a commit to petermetz/scripts that referenced this pull request Jun 7, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
requires rebase This PR requires a git rebase
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants