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

Add support for newer vkd3d-proton + bug fix #2824

Merged
merged 2 commits into from
Apr 24, 2023

Conversation

koplo199
Copy link
Contributor

@koplo199 koplo199 commented Mar 27, 2023

Description

Newer vkd3d-proton will include a new dll : d3d12core.dll.
The d3d12.dll now being a wrapper around this new dll, reference: HansKristian-Work/vkd3d-proton@fbf0382

Additionally, I found why del found[path][dll] was throwing a TypeError: found[path] is a list not a dict, so we must use
found[path].remove(dll) instead.

Type of change

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to not work as expected)
  • This change requires a documentation update

How Has This Been Tested?

Tested locally using bottlesdevs/components#236 allowing to use an up-to-date vkd3d-proton including the new dll.

@Kinsteen
Copy link
Contributor

I'm pretty sure this will break old versions of VKD3D that doesn't have two DLLs... I think this may require a lot more work than this

@github-actions
Copy link
Contributor

github-actions bot commented Mar 27, 2023

Pylint result on modfied files:
************* Module bottles.backend.dlls.dll
bottles/backend/dlls/dll.py:164:61: W0511: TODO: should not be ok but just ignore it for now (fixme)
bottles/backend/dlls/dll.py:89:0: W1405: Quote delimiter ' is inconsistent with the rest of the file (inconsistent-quotes)
bottles/backend/dlls/dll.py:89:0: W1405: Quote delimiter ' is inconsistent with the rest of the file (inconsistent-quotes)
bottles/backend/dlls/dll.py:119:0: W1405: Quote delimiter ' is inconsistent with the rest of the file (inconsistent-quotes)
bottles/backend/dlls/dll.py:119:0: W1405: Quote delimiter ' is inconsistent with the rest of the file (inconsistent-quotes)
bottles/backend/dlls/dll.py:144:0: W1405: Quote delimiter ' is inconsistent with the rest of the file (inconsistent-quotes)
bottles/backend/dlls/dll.py:58:8: C0206: Consider iterating with .items() (consider-using-dict-items)
bottles/backend/dlls/dll.py:51:4: R1710: Either all return statements in a function should return an expression, or none of them should. (inconsistent-return-statements)
bottles/backend/dlls/dll.py:116:8: C0206: Consider iterating with .items() (consider-using-dict-items)
bottles/backend/dlls/dll.py:166:16: W0105: String statement has no effect (pointless-string-statement)
bottles/backend/dlls/dll.py:179:12: W0105: String statement has no effect (pointless-string-statement)
bottles/backend/dlls/dll.py:143:4: R1710: Either all return statements in a function should return an expression, or none of them should. (inconsistent-return-statements)
bottles/backend/dlls/dll.py:20:0: W0611: Unused glob imported from glob (unused-import)
bottles/backend/dlls/dll.py:21:0: W0611: Unused NewType imported from typing (unused-import)
bottles/backend/dlls/dll.py:30:0: W0611: Unused WineBoot imported from bottles.backend.wine.wineboot (unused-import)
************* Module bottles.backend.wine.executor
bottles/backend/wine/executor.py:69:0: W0311: Bad indentation. Found 16 spaces, expected 12 (bad-indentation)
bottles/backend/wine/executor.py:74:0: W0311: Bad indentation. Found 16 spaces, expected 12 (bad-indentation)
bottles/backend/wine/executor.py:79:0: W0311: Bad indentation. Found 16 spaces, expected 12 (bad-indentation)
bottles/backend/wine/executor.py:94:0: C0303: Trailing whitespace (trailing-whitespace)
bottles/backend/wine/executor.py:247:0: W1405: Quote delimiter ' is inconsistent with the rest of the file (inconsistent-quotes)
bottles/backend/wine/executor.py:24:4: R0913: Too many arguments (16/5) (too-many-arguments)
bottles/backend/wine/executor.py:141:4: R1710: Either all return statements in a function should return an expression, or none of them should. (inconsistent-return-statements)
bottles/backend/wine/executor.py:199:8: E1111: Assigning result of a function call, where the function has no return (assignment-from-no-return)
bottles/backend/wine/executor.py:281:8: E1111: Assigning result of a function call, where the function has no return (assignment-from-no-return)
bottles/backend/wine/executor.py:296:8: E1111: Assigning result of a function call, where the function has no return (assignment-from-no-return)
bottles/backend/wine/executor.py:310:8: E1111: Assigning result of a function call, where the function has no return (assignment-from-no-return)
bottles/backend/wine/executor.py:324:8: C0103: Variable name "w" doesn't conform to snake_case naming style (invalid-name)
bottles/backend/wine/executor.py:324:11: C0103: Variable name "h" doesn't conform to snake_case naming style (invalid-name)
bottles/backend/wine/executor.py:353:21: W1202: Use lazy % or % formatting in logging functions (logging-format-interpolation)
bottles/backend/wine/executor.py:356:12: C0103: Variable name "m" doesn't conform to snake_case naming style (invalid-name)
bottles/backend/wine/executor.py:4:0: W0611: Unused NewType imported from typing (unused-import)
bottles/backend/wine/executor.py:17:0: W0611: Unused WineBridge imported from bottles.backend.wine.winebridge (unused-import)

@koplo199
Copy link
Contributor Author

koplo199 commented Mar 27, 2023

I'm pretty sure this will break old versions of VKD3D that doesn't have two DLLs... I think this may require a lot more work than this

More seriously, @Kinsteen you're probably right but I'm still 99.9% sure it will work alright: from what I understand if a dll is missing then it is simply not used. The bug prevented this correct behavior by deleting the whole folder if one of the dll went missing, but with the fix included it won't happen now

@Kinsteen
Copy link
Contributor

In any case we must test before we merge

@koplo199
Copy link
Contributor Author

Yes, I really should set a dev environment for Bottles, but I don't want to mess with my local install: any tip for setting up an isolated dev environment? I looked at the doc but couldn't find any info on that matter

@TheEvilSkeleton
Copy link
Contributor

You could use the Flatpak artifacts. However, I suggest to backup your existing Bottles flatpak files, which are located in ~/.var/app/com.usebottles.bottles. If you want, I can send you the direct link to the artifact that you can download

@koplo199
Copy link
Contributor Author

koplo199 commented Apr 2, 2023

You could use the Flatpak artifacts. However, I suggest to backup your existing Bottles flatpak files, which are located in ~/.var/app/com.usebottles.bottles. If you want, I can send you the direct link to the artifact that you can download

Unfortunately, I can't backup my existing files: there are multiple gigabytes of it and I don't have space left on my disk (hence why I asked if there was a way to set up an isolated dev environment).

I guess I could set up an additional user on my system, and then logging to it when developing/testing, but it is not very convenient (need to migrate existing config, etc).

A way to use another folder than ~/.var/app/com.usebottles.bottles like ~/.var/app/com.usebottles.bottles-devel or so would be perfect. Is there any way to achieve that? By editing a build config maybe?

@mirkobrombin
Copy link
Member

A way to use another folder than ~/.var/app/com.usebottles.bottles like ~/.var/app/com.usebottles.bottles-devel or so would be perfect. Is there any way to achieve that? By editing a build config maybe?

You can edit local source of Bottles to point to a different share path, should be under the globals.py file.

@koplo199
Copy link
Contributor Author

You can edit local source of Bottles to point to a different share path, should be under the globals.py file.

Nice, thanks for the hint. Would you accept a PR that does that automatically when a devel build is detected?

@mirkobrombin
Copy link
Member

I'm not sure that's something you would expect, we may have to explain this

@koplo199
Copy link
Contributor Author

I don't know, imho it makes sense: a dev environment should be as clean as possible. That way, we make sure to replicate a first time use, thus avoiding any problem with caching (dependencies, components, old config file no longer shipped by default, or containing different settings, etc).

I'll investigate for personal use, we'll see

@koplo199
Copy link
Contributor Author

koplo199 commented Apr 15, 2023

I ended up changing:

id: com.usebottles.bottles

to id: com.usebottles.bottles.Devel to achieve a separated Devel environment. I don't know if a manifest could be included in another, but creating a com.usebottles.bottles.Devel.yml including com.usebottles.bottles.yml and just changing its id would be the upstream-able solution.

Anyway, it allowed me to test and fix a bug which happens when switching back and forth between two versions not containing the same dlls.

After someone else is able to confirm it also works correctly on its system, It should be ready to merge.

Copy link
Contributor

@Kinsteen Kinsteen left a comment

Choose a reason for hiding this comment

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

Tested and seems to work great!

@TheEvilSkeleton TheEvilSkeleton merged commit fb5dc46 into bottlesdevs:main Apr 24, 2023
2 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants