-
Notifications
You must be signed in to change notification settings - Fork 45
Added DXVK verb #512
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
Added DXVK verb #512
Conversation
Merge Phoenicis/Scripts:master into ImperatorS79/Scripts:master
updated Hearthstone (download application directly) (PhoenicisOrg#383)
Merge master
Merge recent commits
|
Cannot merge before #511. |
Wine 3.5 is including vulkan-1.dll, so vulkanSDK from LunarG is now only needed for debugging purposes :).
|
This can now be committed ;). |
Engines/Wine/Verbs/DXVK/script.js
Outdated
|
|
||
| remove(this.prefixDirectory + "/TMP/"); | ||
|
|
||
| return this; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Something seems to be wrong with the indentation here (remove... is indented more than return). Usually Codacy should complain about this. Please merge the latest master to see if the error is detected then. If not, I would like to update Codacy to detect this first (so don't fix it yet).
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Check successful...
Engines/Wine/Verbs/DXVK/script.js
Outdated
| print("NOTE: wine version should be greater or equal to 3.5"); | ||
|
|
||
| var setupFile = new Resource() | ||
| .wizard(this._wizard) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
better use this.wizard()
Engines/Wine/Verbs/DXVK/script.js
Outdated
| .get(); | ||
|
|
||
| var setupExtracted = new Extractor() | ||
| .wizard(this._wizard) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
see above
|
The indentation is still not the same everywhere. Let me try to check this in Codacy. |
|
I will fix the indentation detection in #539. |
|
Is it possible to execute a .sh in a verb ? So that I will use the setup_dxvk.sh inside the archive to be sure evrything goes where it should. Ex (from dxvk): |
|
You can use the Java |
|
Won't need it after all, this is ready to be commit I think. |
|
I have updated the ESLint configuration on master. Please update so Codacy uses the up-to-date config. Moreover, we have added |
|
I will review as soon as master is merged and all status checks have passed. |
Merge master
|
The merge you did there doesn't seem to have worked properly. At least it doesn't show any changes. |
|
I do not understand. Why should there be any changes in this pull if the only things I am modifying are the script.js and script.json file ? |
|
You're right. I looked at the merge commit but of course GitHub shows only the changes compared to master if I do not click "show diff"... |
Engines/Wine/Verbs/DXVK/script.js
Outdated
| @@ -0,0 +1,50 @@ | |||
| include(["Engines", "Wine", "Engine", "Object"]); | |||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The includes are wrong. We have changed the IDs to all lower case (so e.g. "engines" instead of "Engines").
Engines/Wine/Verbs/DXVK/script.js
Outdated
| include(["Utils", "Functions", "Filesystem", "Files"]); | ||
|
|
||
| /** | ||
| * Setup DXVK-> https://github.com/doitsujin/dxvk/ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Please use the same JSDoc comment style as the other Verbs.
Engines/Wine/Verbs/DXVK/script.js
Outdated
| } | ||
|
|
||
| this.overrideDLL() | ||
| .set("native", ["d3d11"]) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think you can do the overrideDLL in one call.
|
I do not understand where are those "trailing spaces" |
|
For example in l. 17 there's a space behind |
|
Btw: If you install eslint on your PC, you can fix this kind of errors automatically with |
Simply download the latest DXVK build and install it. Can be useful for some game in terms of performance, but should remain an option (for compatibility)