Fix: gamemode.sh windowrule syntax issue when changing window opacity#1519
Conversation
Beforehand gave the error "invalid field opaque: missing a value" and didn't change the opacity.
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
📝 WalkthroughWalkthroughThe gamemode.sh script transitions from Hyprland state-based mode detection to a filesystem toggle mechanism. Instead of querying the animations:enabled option and applying multiple configuration changes via hyprctl batch commands, it now checks for a lock file to determine gamemode state and switches between regular and gaming configurations using a source command. Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes 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 |
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)
CHANGELOG.md (1)
70-70: Fix typo in warning message."borke" should be "break" or "brick" (more appropriate for display driver context).
📝 Proposed fix
-- nv110-nv140 cards, please install `nvidia-580xx-dkms` before updating your whole system or before rebooting. `nvidia-open-dkms` could potentially borke your display. Goodluck! +- nv110-nv140 cards, please install `nvidia-580xx-dkms` before updating your whole system or before rebooting. `nvidia-open-dkms` could potentially brick your display. Good luck!
🤖 Fix all issues with AI agents
In @CHANGELOG.md:
- Line 52: Change the migration subsection headings from h5 to h4 by replacing
the leading "#####" with "####" for the "Hyprland v0.53.0" and "NVIDIA Legacy
Support" headings (the exact heading text strings) so the markdown heading
hierarchy increments properly; update both occurrences referenced in the diff
(including the second instance around the "NVIDIA Legacy Support" heading).
📜 Review details
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (7)
CHANGELOG.mdConfigs/.config/hypr/windowrules.confConfigs/.local/lib/hyde/gamemode.shConfigs/.local/lib/hyde/system.monitor.shConfigs/.local/lib/hyde/system.update.shConfigs/.local/share/hyde/templates/hypr/windowrules.confConfigs/.local/share/hypr/windowrules.conf
🧰 Additional context used
🪛 markdownlint-cli2 (0.18.1)
CHANGELOG.md
52-52: Heading levels should only increment by one level at a time
Expected: h4; Actual: h5
(MD001, heading-increment)
🔇 Additional comments (6)
Configs/.local/share/hypr/windowrules.conf (1)
40-50: LGTM! Well-structured PiP window rule consolidation.The consolidated
hyde_picture_in_pictureblock effectively replaces multiple discrete rules with a single, maintainable configuration. The monitor-relative positioning ensures consistent placement across different display sizes, and the regex pattern comprehensively matches Picture-in-Picture title variations.Configs/.local/lib/hyde/gamemode.sh (1)
21-21: Excellent fix for the windowrule syntax issue!This change directly resolves issue #1506 by adding the missing value for the
opaquefield and updating to Hyprland v0.53's explicitmatch:classsyntax. The rule now correctly sets all windows to opaque during game mode.Configs/.local/share/hyde/templates/hypr/windowrules.conf (2)
20-30: LGTM! Consistent PiP configuration.The
hyde_picture_in_pictureblock matches the consolidated approach in other config files, ensuring consistency across the template and production configurations.
92-92: Good fix for JetBrains windowrule.The
no_initial_focusparameter correctly uses a single boolean value instead of the duplicate. This aligns with proper Hyprland v0.53 syntax.Configs/.config/hypr/windowrules.conf (2)
20-30: LGTM! Consistent consolidation.The
hyde_picture_in_pictureblock maintains consistency with the other configuration files, providing a unified approach to Picture-in-Picture window management.
92-92: Good fix for JetBrains windowrule.The simplified
no_initial_focusparameter correctly uses a single boolean, matching the fix in the template configuration.
|
I stumbled across the exact same change in |
|
I suggest using |
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In @CHANGELOG.md:
- Line 61: The changelog uses a h5 heading "##### Hyprland v0.53.0" (and a
similar h5 at line 70) immediately after an h3 "### Migration", violating MD001;
change those h5 headings to h4 ("#### Hyprland v0.53.0") so heading levels
increment by one (ensure both occurrences are updated).
📜 Review details
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
CHANGELOG.md
🧰 Additional context used
🪛 markdownlint-cli2 (0.18.1)
CHANGELOG.md
61-61: Heading levels should only increment by one level at a time
Expected: h4; Actual: h5
(MD001, heading-increment)
🔇 Additional comments (1)
CHANGELOG.md (1)
63-66: Changelog migration content looks good.The updates to the Hyprland v0.53.0 and NVIDIA Legacy Support sections provide clear migration guidance aligned with the PR's bug fixes and syntax changes. The messaging is actionable and encourages users to proactively address configuration issues.
Also applies to: 72-88
| ### Migration | ||
|
|
||
| ##### Hyprland | ||
| ##### Hyprland v0.53.0 |
There was a problem hiding this comment.
Fix markdown heading hierarchy violation (MD001).
Heading levels should increment by one level at a time. The migration subsections jump from h3 (### Migration) directly to h5 (##### Hyprland v0.53.0), skipping h4. This violates the MD001 rule and should use h4 (####) instead.
📝 Proposed fix for heading levels
- ##### Hyprland v0.53.0
+ #### Hyprland v0.53.0- ##### NVIDIA Legacy Support
+ #### NVIDIA Legacy SupportAlso applies to: 70-70
🧰 Tools
🪛 markdownlint-cli2 (0.18.1)
61-61: Heading levels should only increment by one level at a time
Expected: h4; Actual: h5
(MD001, heading-increment)
🤖 Prompt for AI Agents
In @CHANGELOG.md at line 61, The changelog uses a h5 heading "##### Hyprland
v0.53.0" (and a similar h5 at line 70) immediately after an h3 "### Migration",
violating MD001; change those h5 headings to h4 ("#### Hyprland v0.53.0") so
heading levels increment by one (ensure both occurrences are updated).
Beforehand using this script would only apply opacity to new windows. Now when using this script it applies opacity automatically.
Great suggestion. |
gamemode.sh is just a reference from the hyprland wiki anyway. That's why I would still recommend our own workflows script. This way the state is saved. gamemode.sh will still stay for reference though. I'll try to fix some conflicts on this PR first |
| @@ -1,25 +1,7 @@ | |||
| #!/usr/bin/env bash | |||
There was a problem hiding this comment.
Please use hyprctl for this. Sorry for the misunderstanding. Thanks!
Thanks for letting us know. I am still new to HyDE and try to understand some of it's inner workings. |
|
This PR is stale because it has been open 45 days with no activity. Remove stale label or comment or this will be closed in 10 days. |
|
Gonna bump before the PR gets closed down. In the meantime new people ran into the same issue and I ended up redirecting them to this PR as a workaround. Depending on what the scope is we could leave it at that or look into a way to remember and restore the preview workflow. However, this could require changes in how we are detecting the |
Add lock file handling to manage gamemode state.
|
Will merge this now Only the best way I can think of for now. |
Pull Request
Description
Fixed a syntax issue in gamemode.sh that caused the script to not change the opacity of windows.
Type of change
Checklist
Summary by CodeRabbit