D7vk support#338
Conversation
|
Important Review skippedDraft detected. Please check the settings in the CodeRabbit UI or the You can disable this status message by setting the Use the checkbox below for a quick retry:
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthroughAdds first-class D7VK support: UI state and dropdown, resource entries and default version, content-type enum, dxwrapper normalization/extraction updates, and container eligibility for Changes
Sequence Diagram(s)sequenceDiagram
autonumber
participant UI as ContainerConfigDialog (UI)
participant State as ContainerConfigState
participant Setup as XServerScreen
participant Container as Container core
participant FS as Filesystem
Note over UI,State: User selects wrapper and per-wrapper version
UI->>State: update dxwrapperConfig (e.g., "d7vk-1.1") and d7vkVersionIndex
State-->>UI: state updated
alt dxwrapper starts with "d7vk-"
UI->>Setup: setupWineSystemFiles(dxwrapper)
Setup->>State: read/validate d7vk version (fallback DefaultVersion.D7VK)
Setup->>Container: request extraction for dxwrapper (d7vk-*)
Container->>FS: extract DXVK/D7VK/D8VK DLLs / apply content profile
FS-->>Container: extraction result
Container-->>Setup: extraction applied
else other dxwrapper variants
UI->>Setup: setupWineSystemFiles(dxwrapper=...)
Setup->>Container: request extraction for dxwrapper
end
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
|
Going to update this PR tomorrow to add the 1.1 instead as that has large improvements over 1.0 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
app/src/main/java/app/gamenative/ui/component/dialog/ContainerConfigDialog.kt (1)
41-47: Duplicate import ofTextButton.
TextButtonis imported twice (lines 41 and 47). Remove one of the duplicates.🔎 Proposed fix
import androidx.compose.material3.TextButton import androidx.compose.material3.OutlinedTextField import androidx.compose.material3.Scaffold import androidx.compose.material3.Slider import androidx.compose.material3.CircularProgressIndicator import androidx.compose.material3.Text -import androidx.compose.material3.TextButton
🧹 Nitpick comments (1)
app/src/main/java/app/gamenative/ui/component/dialog/ContainerConfigDialog.kt (1)
482-516: Consider usingDefaultVersionconstants instead of hardcoded index 0.When resetting to a default version, using
DefaultVersion.D7VKandDefaultVersion.DXVKwould be more robust and consistent with how defaults are handled elsewhere in the codebase (e.g., line 709 usesDefaultVersion.DXVK).🔎 Proposed fix
when (config.dxwrapper) { "d7vk" -> { // Check if version is valid for D7VK val isValidD7VKVersion = d7vkVersionsAll.any { StringUtils.parseIdentifier(it) == savedVersion } if (!isValidD7VKVersion && d7vkVersionsAll.isNotEmpty()) { - // Reset to first D7VK version - val defaultVersion = StringUtils.parseIdentifier(d7vkVersionsAll[0]) + // Reset to default D7VK version + val defaultIdx = d7vkVersionsAll.indexOfFirst { + StringUtils.parseIdentifier(it) == DefaultVersion.D7VK + }.coerceAtLeast(0) + val defaultVersion = StringUtils.parseIdentifier(d7vkVersionsAll[defaultIdx]) currentConfig.put("version", defaultVersion) config = config.copy(dxwrapperConfig = currentConfig.toString()) - d7vkVersionIndex = 0 + d7vkVersionIndex = defaultIdx } } "dxvk" -> { // Check if version is valid for DXVK val isValidDXVKVersion = dxvkVersionsAll.any { StringUtils.parseIdentifier(it) == savedVersion } if (!isValidDXVKVersion && dxvkVersionsAll.isNotEmpty()) { - // Reset to first DXVK version - val defaultVersion = StringUtils.parseIdentifier(dxvkVersionsAll[0]) + // Reset to default DXVK version + val defaultIdx = dxvkVersionsAll.indexOfFirst { + StringUtils.parseIdentifier(it) == DefaultVersion.DXVK + }.coerceAtLeast(0) + val defaultVersion = StringUtils.parseIdentifier(dxvkVersionsAll[defaultIdx]) currentConfig.put("version", version) config = config.copy(dxwrapperConfig = currentConfig.toString()) - dxvkVersionIndex = 0 + dxvkVersionIndex = defaultIdx } } }
📜 Review details
Configuration used: defaults
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (7)
app/src/main/assets/dxwrapper/d7vk-1.1.tzstapp/src/main/java/app/gamenative/ui/component/dialog/ContainerConfigDialog.ktapp/src/main/java/app/gamenative/ui/screen/xserver/XServerScreen.ktapp/src/main/java/com/winlator/container/Container.javaapp/src/main/java/com/winlator/contents/ContentProfile.javaapp/src/main/java/com/winlator/core/DefaultVersion.javaapp/src/main/res/values/arrays.xml
🧰 Additional context used
🧬 Code graph analysis (1)
app/src/main/java/app/gamenative/ui/component/dialog/ContainerConfigDialog.kt (3)
app/src/main/java/com/winlator/core/KeyValueSet.java (1)
KeyValueSet(7-169)app/src/main/java/app/gamenative/ui/component/settings/SettingsListDropdown.kt (1)
SettingsListDropdown(40-125)app/src/main/java/app/gamenative/ui/theme/Color.kt (1)
settingsTileColors(27-32)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: build
🔇 Additional comments (8)
app/src/main/java/com/winlator/core/DefaultVersion.java (1)
18-18: LGTM!The D7VK version constant follows the established pattern and is consistent with other wrapper version constants in the class.
app/src/main/java/com/winlator/contents/ContentProfile.java (1)
28-28: LGTM!The D7VK content type enum addition is properly placed alongside other DirectX wrapper types and follows the established naming convention.
app/src/main/java/com/winlator/container/Container.java (1)
850-850: LGTM!The extension of the dxwrapper eligibility check to include
d7vk-*prefixes is consistent with the existing handling ofd8vk-*anddxvk-*variants.app/src/main/res/values/arrays.xml (1)
79-79: LGTM!The D7VK resource entries are properly integrated:
- "D7VK" added to wrapper selection list
- Version array initialized with "1.1" (matching DefaultVersion.D7VK)
- Logical placement alongside other wrapper options
Also applies to: 83-85
app/src/main/java/app/gamenative/ui/screen/xserver/XServerScreen.kt (2)
2145-2155: Verify the intentional version enforcement for D7VK.The D7VK version normalization logic differs from the DXVK implementation:
- D7VK (lines 2145-2155): Forces
DefaultVersion.D7VK("1.1") when the version is null or doesn't match the default- DXVK (lines 2140-2144): Simply appends the version from config without validation
This inconsistency means:
- Users cannot select custom D7VK versions (e.g., future 1.2 or custom builds)
- Any existing D7VK 1.0 configurations will be automatically upgraded to 1.1
If this forced versioning is intentional (e.g., to ensure users benefit from 1.1 improvements mentioned in the PR), consider:
- Adding a comment explaining why D7VK differs from DXVK
- Evaluating whether DXVK should have similar validation
If unintentional, align with DXVK behavior by removing the
version != DefaultVersion.D7VKcondition.🔎 Suggested refactor to align with DXVK behavior
if (xServerState.value.dxwrapper == "d7vk") { - // Set version for d7vk or use default if not found. - var version = xServerState.value.dxwrapperConfig?.get("version") - if (version == null || version != DefaultVersion.D7VK) { - version = DefaultVersion.D7VK - xServerState.value.dxwrapperConfig?.put("version", version) - } xServerState.value = xServerState.value.copy( - dxwrapper = "d7vk-" + version, + dxwrapper = "d7vk-" + xServerState.value.dxwrapperConfig?.get("version"), ) }
2335-2341: LGTM!The updated comments and log messages appropriately reflect the expanded support for D7VK alongside DXVK and D8VK wrappers.
app/src/main/java/app/gamenative/ui/component/dialog/ContainerConfigDialog.kt (2)
149-149: LGTM!The D7VK version list initialization follows the established pattern used for DXVK and VKD3D, correctly initializing base versions from resources, creating mutable state, and augmenting with installed content profiles.
Also applies to: 191-191, 231-231
714-742: LGTM!Good addition of the guard clause to ensure this DXVK-specific async version handling only runs when the DXVK wrapper is selected, preventing unintended side effects when D7VK or other wrappers are active.
|
Will do some final tests to ensure that everything is working correctly. |
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Fix all issues with AI agents
In
@app/src/main/java/app/gamenative/ui/component/dialog/ContainerConfigDialog.kt:
- Around line 591-597: Remove the composition-time mutation of config: delete
the else-if block that checks isVKD3D and writes version into
config.dxwrapperConfig (the block that constructs
KeyValueSet(config.dxwrapperConfig), puts "version", and assigns config =
config.copy(...)). This logic is already handled in
LaunchedEffect(graphicsDriverIndex, dxWrapperIndex), so stop performing
side-effectful updates to config during composition (remove references to
isVKD3D/isVortekLike in that block).
- Around line 535-554: The code sets version fields by directly indexing
dxvkVersionsAll[0] and d7vkVersionsAll[0] which can throw
IndexOutOfBoundsException; update the dxwrapper switch in
ContainerConfigDialog.kt (symbols: dxvkVersionIndex, d7vkVersionIndex,
dxvkVersionsAll, d7vkVersionsAll, currentConfig.put) to first check that the
respective list is not empty (e.g., if (dxvkVersionsAll.isNotEmpty()) {
dxvkVersionIndex = 0; val newVersion =
StringUtils.parseIdentifier(dxvkVersionsAll[0]); currentConfig.put("version",
newVersion) } else { /* set a safe default or skip putting version */ }) and do
the equivalent for d7vkVersionsAll so you only parse and store index 0 when
present and avoid updating the index when the list is empty.
📜 Review details
Configuration used: defaults
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
app/src/main/java/app/gamenative/ui/component/dialog/ContainerConfigDialog.ktapp/src/main/res/values/strings.xml
🔇 Additional comments (7)
app/src/main/res/values/strings.xml (1)
117-117: LGTM! String resource properly added.The D7VK version string resource follows the same pattern as the existing DXVK version string (line 116) and is correctly placed in the Win Components section for proper internationalization.
app/src/main/java/app/gamenative/ui/component/dialog/ContainerConfigDialog.kt (6)
149-149: LGTM! D7VK version lists properly initialized.The D7VK version base and runtime lists follow the same pattern as DXVK versions, ensuring consistency across wrapper implementations.
Also applies to: 191-191
231-231: LGTM! D7VK content profiles properly loaded.The D7VK versions are correctly augmented with installed content profiles, following the same pattern as DXVK and VKD3D.
479-479: LGTM! D7VK version index state properly initialized.The D7VK version index state variable follows the same pattern as the DXVK version index.
482-516: Good addition, but bounds checking still needed.The validation logic properly fixes version mismatches when loading containers, which prevents crashes from stale or invalid version references. However, the bounds checking issue flagged in past reviews (lines 494-500 and 507-513) still applies: accessing
d7vkVersionsAll[0]ordxvkVersionsAll[0]without checking for empty lists can throwIndexOutOfBoundsException.Add defensive checks before accessing index 0:
"d7vk" -> { val isValidD7VKVersion = d7vkVersionsAll.any { StringUtils.parseIdentifier(it) == savedVersion } if (!isValidD7VKVersion && d7vkVersionsAll.isNotEmpty()) { val defaultVersion = StringUtils.parseIdentifier(d7vkVersionsAll[0]) currentConfig.put("version", defaultVersion) config = config.copy(dxwrapperConfig = currentConfig.toString()) d7vkVersionIndex = 0 } else if (d7vkVersionsAll.isEmpty()) { // Handle empty list case - perhaps log warning or use a hardcoded default timber.log.Timber.w("D7VK versions list is empty, cannot reset version") } }Similar pattern for the "dxvk" case.
599-618: LGTM! D7VK version dropdown properly implemented.The D7VK version dropdown is correctly implemented with:
- Proper use of string resource for the title (line 606)
- Bounds checking via
coerceIn(line 607)- Correct state management and config updates
- Conditional visibility based on wrapper selection
715-743: Good improvement to scope DXVK version handling.The addition of the wrapper type check (lines 715-718) ensures DXVK version logic only runs when the DXVK wrapper is actually selected, preventing unintended version updates for D7VK or VKD3D wrappers. This makes the version management more robust and prevents conflicts between different wrapper types.
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
app/src/main/java/app/gamenative/ui/component/dialog/ContainerConfigDialog.kt (1)
706-715: Missing D7VK version index synchronization.DXVK has this
LaunchedEffectto sync the version index fromconfig.dxwrapperConfigwhen the dialog loads, ensuring the UI displays the correct saved version. D7VK lacks equivalent logic. When the dialog loads with a saved D7VK container,d7vkVersionIndexremains at its initial value of 0 (line 479) instead of reflecting the actual saved version, causing the dropdown at line 610 to display the first version in the list rather than the configured version.🔄 Proposed fix: Add D7VK version index sync
Add a similar effect for D7VK after line 715:
LaunchedEffect(versionsLoaded, d7vkVersionsAll, config.dxwrapper, config.dxwrapperConfig) { if (!versionsLoaded) return@LaunchedEffect if (config.dxwrapper != "d7vk") return@LaunchedEffect val kvs = KeyValueSet(config.dxwrapperConfig) val configuredVersion = kvs.get("version") val foundIndex = d7vkVersionsAll.indexOfFirst { StringUtils.parseIdentifier(it) == configuredVersion } val defaultIndex = d7vkVersionsAll.indexOfFirst { StringUtils.parseIdentifier(it) == DefaultVersion.D7VK }.coerceAtLeast(0) val newIdx = if (foundIndex >= 0) foundIndex else defaultIndex if (d7vkVersionIndex != newIdx) d7vkVersionIndex = newIdx }
🤖 Fix all issues with AI agents
In
@app/src/main/java/app/gamenative/ui/component/dialog/ContainerConfigDialog.kt:
- Around line 595-600: The code block that builds and mutates `config` using
`val version = if (isVortekLike) "1.10.3" else "2.4.1"`,
`KeyValueSet(config.dxwrapperConfig)` and `config = config.copy(dxwrapperConfig
= currentConfig.toString())` must be removed because it performs a
composition-side effect that is already handled in
`LaunchedEffect(graphicsDriverIndex, dxWrapperIndex)` and incorrectly runs for
D7VK; delete that entire else-branch so DXVK/VKD3D versioning is only set by the
existing `LaunchedEffect` and D7VK retains its own handling in the D7VK-specific
code paths.
📜 Review details
Configuration used: defaults
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
app/src/main/java/app/gamenative/ui/component/dialog/ContainerConfigDialog.ktapp/src/main/java/app/gamenative/ui/screen/xserver/XServerScreen.ktapp/src/main/res/values/strings.xml
🚧 Files skipped from review as they are similar to previous changes (2)
- app/src/main/res/values/strings.xml
- app/src/main/java/app/gamenative/ui/screen/xserver/XServerScreen.kt
🧰 Additional context used
🧬 Code graph analysis (1)
app/src/main/java/app/gamenative/ui/component/dialog/ContainerConfigDialog.kt (3)
app/src/main/java/com/winlator/core/KeyValueSet.java (1)
KeyValueSet(7-169)app/src/main/java/app/gamenative/ui/component/settings/SettingsListDropdown.kt (1)
SettingsListDropdown(40-125)app/src/main/java/app/gamenative/ui/theme/Color.kt (1)
settingsTileColors(27-32)
🔇 Additional comments (4)
app/src/main/java/app/gamenative/ui/component/dialog/ContainerConfigDialog.kt (4)
149-149: LGTM: D7VK state initialization follows established patterns.The D7VK version lists and state variables are initialized consistently with the existing DXVK implementation.
Also applies to: 191-191, 231-231, 479-479
482-516: LGTM: Validation logic properly handles empty version lists.The bounds checks (
isNotEmpty()) correctly preventIndexOutOfBoundsExceptionwhen version lists are empty, addressing concerns from previous reviews.
535-558: LGTM: Wrapper switching safely resets versions with bounds checking.The addition of
isNotEmpty()guards before accessing[0]correctly addresses theIndexOutOfBoundsExceptionconcerns raised in previous reviews.
602-621: LGTM: D7VK version dropdown properly implemented.The dropdown uses proper string resources (addressing the previous review comment) and safely handles bounds with
coerceIn. The version selection logic correctly updatesdxwrapperConfig.
… into d7vk-support
Also hide the dxvk version when d7vk is selected.
![cubic-dev-ai[bot]](https://avatars.githubusercontent.com/in/1082092?s=60&v=4){kind=small}
There was a problem hiding this comment.
1 issue found across 10 files
Prompt for AI agents (all issues)
Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.
<file name="app/src/main/java/app/gamenative/ui/screen/xserver/XServerScreen.kt">
<violation number="1" location="app/src/main/java/app/gamenative/ui/screen/xserver/XServerScreen.kt:2769">
P2: D7VK normalization overwrites any non-default version, so user-selected D7VK versions are reset to the default and ignored during extraction.</violation>
</file>
Reply with feedback, questions, or to request a fix. Tag @cubic-dev-ai to re-run a review.
There was a problem hiding this comment.
1 issue found across 4 files (changes from recent commits).
Prompt for AI agents (all issues)
Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.
<file name="app/src/main/java/app/gamenative/ui/component/dialog/GraphicsTab.kt">
<violation number="1" location="app/src/main/java/app/gamenative/ui/component/dialog/GraphicsTab.kt:348">
P2: D7VK now hides the DXVK dropdown, but the hidden-branch still overwrites `dxwrapperConfig.version` with a DXVK default. This runs when D7VK is selected, so the D7VK version can’t persist and the UI reads the wrong default.</violation>
</file>
Reply with feedback, questions, or to request a fix. Tag @cubic-dev-ai to re-run a review.
There was a problem hiding this comment.
Actionable comments posted: 4
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In
`@app/src/main/java/app/gamenative/ui/component/dialog/ContainerConfigDialog.kt`:
- Around line 677-679: Change the incorrect delegated declaration for
d7vkVersionIndexRef to an assigned MutableState so it matches the dxvk pattern:
replace "var d7vkVersionIndexRef by rememberSaveable { mutableIntStateOf(0) }"
with "val d7vkVersionIndexRef = rememberSaveable { mutableIntStateOf(0) }" so
that the subsequent "var d7vkVersionIndex by d7vkVersionIndexRef" can validly
use the state delegate; keep the rememberSaveable and mutableIntStateOf calls
unchanged.
- Around line 677-679: d7vkVersionIndexRef and d7vkVersionIndex are declared but
never used; either remove them or wire them into ContainerConfigState like
dxvkVersionIndex. Add a LaunchedEffect that consumes d7vkVersionIndexRef
(mirroring the dxvk LaunchedEffect pattern) and set d7vkVersionIndex =
d7vkVersionIndexRef where ContainerConfigState is constructed (similar to
dxvkVersionIndex = dxvkVersionIndexRef), or simply delete both
d7vkVersionIndexRef/d7vkVersionIndex declarations if D7VK version tracking isn’t
needed.
In `@app/src/main/java/app/gamenative/ui/component/dialog/GraphicsTab.kt`:
- Around line 455-473: The build fails because DefaultVersion is unresolved in
GraphicsTab.kt; fix it by adding the appropriate import for DefaultVersion (the
enum/object used for the default "D7VK" version) at the top of the file where
other imports are declared—locate DefaultVersion in the codebase (or via your
IDE) and import its declaring package so the reference in the D7VK Version UI
(used in the SettingsListDropdown logic) compiles.
In `@app/src/main/java/app/gamenative/ui/screen/xserver/XServerScreen.kt`:
- Around line 2766-2775: The dxwrapper version logic currently forces the
default for any non-matching version; change it to only apply the default when
the stored value is missing or blank: check
xServerState.value.dxwrapperConfig?.get("version") with isNullOrBlank() (or
equivalent) and only then set version = DefaultVersion.D7VK and put it into
dxwrapperConfig; otherwise preserve the existing non-empty version and continue
to set xServerState.value = xServerState.value.copy(dxwrapper = "d7vk-" +
version).
ℹ️ Review info
Configuration used: defaults
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (7)
app/src/main/assets/dxwrapper/d7vk-1.3.tzstapp/src/main/java/app/gamenative/ui/component/dialog/ContainerConfigDialog.ktapp/src/main/java/app/gamenative/ui/component/dialog/ContainerConfigState.ktapp/src/main/java/app/gamenative/ui/component/dialog/GraphicsTab.ktapp/src/main/java/app/gamenative/ui/screen/xserver/XServerScreen.ktapp/src/main/java/com/winlator/container/Container.javaapp/src/main/java/com/winlator/core/DefaultVersion.java
🚧 Files skipped from review as they are similar to previous changes (1)
- app/src/main/java/com/winlator/core/DefaultVersion.java
| var d7vkVersionIndexRef by rememberSaveable { mutableIntStateOf(0) } | ||
| var d7vkVersionIndex by d7vkVersionIndexRef | ||
|
|
There was a problem hiding this comment.
Build failure: incorrect delegation syntax for d7vkVersionIndexRef.
var d7vkVersionIndexRef by rememberSaveable { mutableIntStateOf(0) } uses Kotlin property delegation — by causes d7vkVersionIndexRef to resolve to Int (the unwrapped value), not MutableState<Int>. The subsequent var d7vkVersionIndex by d7vkVersionIndexRef then tries to use a plain Int as a property delegate, which has no getValue method, producing the exact pipeline error:
Property delegate must have a 'getValue(Nothing?, KMutableProperty0<*>)' method.
The correct pattern, already used for the analogous dxvkVersionIndexRef at lines 674–675, is val … = (assignment), not var … by (delegation):
🐛 Proposed fix
- var d7vkVersionIndexRef by rememberSaveable { mutableIntStateOf(0) }
+ val d7vkVersionIndexRef = rememberSaveable { mutableIntStateOf(0) }
var d7vkVersionIndex by d7vkVersionIndexRef📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| var d7vkVersionIndexRef by rememberSaveable { mutableIntStateOf(0) } | |
| var d7vkVersionIndex by d7vkVersionIndexRef | |
| val d7vkVersionIndexRef = rememberSaveable { mutableIntStateOf(0) } | |
| var d7vkVersionIndex by d7vkVersionIndexRef |
🧰 Tools
🪛 GitHub Actions: Pull Request build check
[error] 678-678: Command './gradlew :app:testDebugUnitTest' - Property delegate must have a 'getValue(Nothing?, KMutableProperty0<*>)' method.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In
`@app/src/main/java/app/gamenative/ui/component/dialog/ContainerConfigDialog.kt`
around lines 677 - 679, Change the incorrect delegated declaration for
d7vkVersionIndexRef to an assigned MutableState so it matches the dxvk pattern:
replace "var d7vkVersionIndexRef by rememberSaveable { mutableIntStateOf(0) }"
with "val d7vkVersionIndexRef = rememberSaveable { mutableIntStateOf(0) }" so
that the subsequent "var d7vkVersionIndex by d7vkVersionIndexRef" can validly
use the state delegate; keep the rememberSaveable and mutableIntStateOf calls
unchanged.
🧩 Analysis chain
🏁 Script executed:
rg -n "d7vkVersionIndex" --type=kt -C3Repository: utkarshdalal/GameNative
Length of output: 93
🏁 Script executed:
rg "d7vkVersionIndex" --type-add 'kt:*.kt' --type kt -C3Repository: utkarshdalal/GameNative
Length of output: 1053
🏁 Script executed:
rg "d7vkVersionIndex" --type-add 'kt:*.kt' --type ktRepository: utkarshdalal/GameNative
Length of output: 350
🏁 Script executed:
# Find ContainerConfigState definition
rg -n "class ContainerConfigState|data class ContainerConfigState" --type-add 'kt:*.kt' --type kt -A 20Repository: utkarshdalal/GameNative
Length of output: 2817
🏁 Script executed:
# Get full ContainerConfigState class definition
rg -n "class ContainerConfigState" --type-add 'kt:*.kt' --type kt -A 50 | head -80Repository: utkarshdalal/GameNative
Length of output: 6622
🏁 Script executed:
rg "d7vkVersionIndex" app/src/main/java/app/gamenative/ui/component/dialog/ContainerConfigState.ktRepository: utkarshdalal/GameNative
Length of output: 49
🏁 Script executed:
# Check lines 722-776 for LaunchedEffect pattern
sed -n '722,776p' app/src/main/java/app/gamenative/ui/component/dialog/ContainerConfigDialog.ktRepository: utkarshdalal/GameNative
Length of output: 3282
🏁 Script executed:
# Check lines 960-975 for state construction
sed -n '960,975p' app/src/main/java/app/gamenative/ui/component/dialog/ContainerConfigDialog.ktRepository: utkarshdalal/GameNative
Length of output: 916
🏁 Script executed:
# Search for d7vkVersionIndex in state construction context
rg -n "d7vkVersionIndex" app/src/main/java/app/gamenative/ui/component/dialog/ContainerConfigDialog.kt -B2 -A2Repository: utkarshdalal/GameNative
Length of output: 334
🏁 Script executed:
# Look for state construction section with d7vkVersionIndex
rg -n "d7vkVersionIndex.*=" app/src/main/java/app/gamenative/ui/component/dialog/ContainerConfigDialog.ktRepository: utkarshdalal/GameNative
Length of output: 121
🏁 Script executed:
# Search for LaunchedEffect with d7vk
rg -n "LaunchedEffect.*d7vk|d7vk.*LaunchedEffect" app/src/main/java/app/gamenative/ui/component/dialog/ContainerConfigDialog.kt -B2 -A15Repository: utkarshdalal/GameNative
Length of output: 49
🏁 Script executed:
# Search for any d7vk references
rg -n "d7vk" app/src/main/java/app/gamenative/ui/component/dialog/ContainerConfigDialog.kt -B2 -A2Repository: utkarshdalal/GameNative
Length of output: 1536
🏁 Script executed:
# Search for where ContainerConfigState is instantiated
rg -n "ContainerConfigState(" app/src/main/java/app/gamenative/ui/component/dialog/ContainerConfigDialog.kt -A 70 | grep -E "^\s*[a-zA-Z]+.*=\s*(.*Ref|.*Index)"Repository: utkarshdalal/GameNative
Length of output: 159
🏁 Script executed:
# Find ContainerConfigState instantiation
rg -n "ContainerConfigState\(" app/src/main/java/app/gamenative/ui/component/dialog/ContainerConfigDialog.kt -A 70Repository: utkarshdalal/GameNative
Length of output: 4153
🏁 Script executed:
# Get more lines to see if d7vkVersionIndex appears later in state construction
sed -n '932,1010p' app/src/main/java/app/gamenative/ui/component/dialog/ContainerConfigDialog.kt | grep -i "d7vk\|dxvk"Repository: utkarshdalal/GameNative
Length of output: 267
🏁 Script executed:
# Search for d7vk references in GraphicsTab
rg -n "d7vk" app/src/main/java/app/gamenative/ui/component/graphics/GraphicsTab.kt -B2 -A2Repository: utkarshdalal/GameNative
Length of output: 176
d7vkVersionIndex is unused and not wired into ContainerConfigState.
The variable is declared at lines 677-678 but never referenced elsewhere in the file. Unlike dxvkVersionIndex, it has no LaunchedEffect consuming it (compare to lines 722-776) and is not passed to the ContainerConfigState constructor (compare dxvkVersionIndex = dxvkVersionIndexRef at line 967). Either remove the unused variable or wire it into the state following the same pattern as dxvkVersionIndex.
🧰 Tools
🪛 GitHub Actions: Pull Request build check
[error] 678-678: Command './gradlew :app:testDebugUnitTest' - Property delegate must have a 'getValue(Nothing?, KMutableProperty0<*>)' method.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In
`@app/src/main/java/app/gamenative/ui/component/dialog/ContainerConfigDialog.kt`
around lines 677 - 679, d7vkVersionIndexRef and d7vkVersionIndex are declared
but never used; either remove them or wire them into ContainerConfigState like
dxvkVersionIndex. Add a LaunchedEffect that consumes d7vkVersionIndexRef
(mirroring the dxvk LaunchedEffect pattern) and set d7vkVersionIndex =
d7vkVersionIndexRef where ContainerConfigState is constructed (similar to
dxvkVersionIndex = dxvkVersionIndexRef), or simply delete both
d7vkVersionIndexRef/d7vkVersionIndex declarations if D7VK version tracking isn’t
needed.
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (2)
app/src/main/java/app/gamenative/ui/component/dialog/ContainerConfigDialog.kt (2)
677-678:⚠️ Potential issue | 🔴 Critical
d7vkVersionIndexRefstill usesby(delegation) instead of=(assignment), leaving it an unwrappedInt.
by rememberSaveable { mutableIntStateOf(0) }callsgetValue()on theMutableIntState, sod7vkVersionIndexRefis of typeInt, notMutableState<Int>. Consequently:
d7vkVersionIndexat line 678 is a plainIntcopy — mutations to it don't trigger recomposition.- Neither can be passed as a
MutableState<Int>ref toContainerConfigState, preventing D7VK version tracking from working at all.The correct pattern mirrors
dxvkVersionIndexRefat lines 674–675:🐛 Proposed fix
- var d7vkVersionIndexRef by rememberSaveable { mutableIntStateOf(0) } - var d7vkVersionIndex = d7vkVersionIndexRef + val d7vkVersionIndexRef = rememberSaveable { mutableIntStateOf(0) } + var d7vkVersionIndex by d7vkVersionIndexRef🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@app/src/main/java/app/gamenative/ui/component/dialog/ContainerConfigDialog.kt` around lines 677 - 678, d7vkVersionIndexRef is currently declared with delegation (by rememberSaveable { mutableIntStateOf(0) }) which yields an Int not a MutableState, so d7vkVersionIndex becomes a plain Int and won't recompose or be passable to ContainerConfigState; change the declaration to mirror dxvkVersionIndexRef (i.e., remove the "by" and assign the result of rememberSaveable { mutableIntStateOf(0) } to get a MutableIntState), then use that MutableIntState (and not the copied Int) when passing the D7VK version ref into ContainerConfigState so version changes trigger recomposition and state is mutable.
939-939:⚠️ Potential issue | 🔴 Critical
d7vkVersionIndexRefis not wired intoContainerConfigState.Line 939 correctly passes
d7vkVersionsAllRef, but after fixing theby→=issue at line 677, the resultingMutableIntStatealso needs to be forwarded — matching thedxvkVersionIndex = dxvkVersionIndexRefparameter at line 967. Without it,GraphicsTab.kt(and any other downstream consumer ofContainerConfigState) cannot read or persist the selected D7VK version index.// In ContainerConfigState constructor call, alongside line 967: + d7vkVersionIndex = d7vkVersionIndexRef,Also applies to: 967-967
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@app/src/main/java/app/gamenative/ui/component/dialog/ContainerConfigDialog.kt` at line 939, ContainerConfigState is missing the d7vkVersionIndex binding so the MutableIntState d7vkVersionIndexRef isn't forwarded; in the ContainerConfigState constructor call (where d7vkVersionsAll = d7vkVersionsAllRef and dxvkVersionIndex = dxvkVersionIndexRef are passed) add d7vkVersionIndex = d7vkVersionIndexRef so GraphicsTab.kt and other consumers can read and persist the selected D7VK index.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@app/src/main/java/com/winlator/core/envvars/EnvVarInfo.kt`:
- Around line 212-219: Remove the incorrect "D7VK_HUD" EnvVarInfo entry from
KNOWN_ENV_VARS (the map entry with identifier "D7VK_HUD" and selectionType
EnvVarSelectionType.MULTI_SELECT) so the HUD is controlled solely by the
existing "DXVK_HUD" entry; also remove any code that sets the D7VK_HUD
environment variable at runtime (where XServerScreen sets env vars) to avoid
silently ignored configuration and rely on DXVK_HUD instead.
---
Duplicate comments:
In
`@app/src/main/java/app/gamenative/ui/component/dialog/ContainerConfigDialog.kt`:
- Around line 677-678: d7vkVersionIndexRef is currently declared with delegation
(by rememberSaveable { mutableIntStateOf(0) }) which yields an Int not a
MutableState, so d7vkVersionIndex becomes a plain Int and won't recompose or be
passable to ContainerConfigState; change the declaration to mirror
dxvkVersionIndexRef (i.e., remove the "by" and assign the result of
rememberSaveable { mutableIntStateOf(0) } to get a MutableIntState), then use
that MutableIntState (and not the copied Int) when passing the D7VK version ref
into ContainerConfigState so version changes trigger recomposition and state is
mutable.
- Line 939: ContainerConfigState is missing the d7vkVersionIndex binding so the
MutableIntState d7vkVersionIndexRef isn't forwarded; in the ContainerConfigState
constructor call (where d7vkVersionsAll = d7vkVersionsAllRef and
dxvkVersionIndex = dxvkVersionIndexRef are passed) add d7vkVersionIndex =
d7vkVersionIndexRef so GraphicsTab.kt and other consumers can read and persist
the selected D7VK index.
ℹ️ Review info
Configuration used: defaults
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (4)
app/src/main/java/app/gamenative/ui/component/dialog/ContainerConfigDialog.ktapp/src/main/java/app/gamenative/ui/component/dialog/GraphicsTab.ktapp/src/main/java/app/gamenative/ui/screen/xserver/XServerScreen.ktapp/src/main/java/com/winlator/core/envvars/EnvVarInfo.kt
🚧 Files skipped from review as they are similar to previous changes (2)
- app/src/main/java/app/gamenative/ui/component/dialog/GraphicsTab.kt
- app/src/main/java/app/gamenative/ui/screen/xserver/XServerScreen.kt
|
Had a chat with folk who've used d7vk. Turns out I misread the readme. Instead of extracting to syswow64, we need to extract it to the game folder. We should be careful in that we remove it on container termination to avoid breaking games. |
… it up on exiting container.
1 participant
Added D7VK Support
This PR adds support for D7VK as a graphics wrapper option alongside DXVK and VKD3D.
What is D7VK?
D7VK is a Vulkan-based translation layer for DirectX 7/8/9, designed to improve compatibility and performance for older Windows games on ARM devices. Unlike DXVK (which focuses on DirectX 10/11), D7VK specifically targets legacy DirectX APIs commonly used by games from the late 1990s to mid-2000s.
Benefits (According to D7VK Repository):
Changes:
Note: D7VK is still in a decently early stage of development, so games will still be a while before in a good state.
Summary by CodeRabbit
New Features
Bug Fixes
Resources