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

Tweak tweaks #355

Closed
wants to merge 1 commit into from
Closed

Tweak tweaks #355

wants to merge 1 commit into from

Conversation

Lucki
Copy link
Contributor

@Lucki Lucki commented Mar 22, 2020

Move dxvk options into its own file and fill both with a more complete option set.

Including all of these will clutter the tweak window a lot so this needs some sort of grouping before merging.
While writing I've noticed the two options 1 and full which both can function as a master switch to hide the underlying grouped options like a sane default. Related to #326 (comment)

I can't get PROTON_LOG=1 to work. The closest I get was by also setting SteamGameId=gamehub but this prevents proton from running without the steam client open.

@tkashkin
Copy link
Owner

tkashkin commented Jul 8, 2020

I'm thinking about new tweaks format and that's what I came up with:

{
	"id": "dxvk_hud",
	"name": "DXVK: Show HUD",
	"description": "Enable DXVK HUD",
	"url": "https://github.com/doitsujin/dxvk#hud",
	"group": "DXVK",
	"applicable_to": {
		"platforms": ["windows"],
		"compat": ["wine", "proton"]
	},
	"requires": {
		"kmod": ["some_kernel_module", "or_alternative"],
		"executable": ["some_executable", "or_alternative"]
	},
	"env": {
		"DXVK_HUD": "${option:hud}"
	},
	"command": "some_executable --option ${option:some_option} ${command}",
	"options": {
		"hud": {
			"description": "HUD elements",
			"type": "list",
			"separator": ",",
			"values": {
				"devinfo": "Show name of GPU and driver version",
				"fps": "Show frame rate",
				"frametimes": "Show a frame time graph",
				"submissions": "Show number of command buffers submitted per frame",
				"drawcalls": "Show number of draw calls and render passes per frame",
				"pipelines": "Show total number of graphics and compute pipelines",
				"memory": "Show amount of device memory allocated and used",
				"gpuload": "Show estimated GPU load. May be inaccurate",
				"version": "Show DXVK version",
				"api": "Show D3D feature level used by the application",
				"compiler": "Show shader compiler activity",
				"samplers": "Show current number of sampler pairs used [D3D9 Only]"
			},
			"presets": {
				"default": {
					"name": "Default",
					"description": "Show name of GPU, driver version and frame rate",
					"value": "devinfo,fps"
				},
				"full": {
					"name": "Full",
					"description": "Show all available HUD elements",
					"value": "full"
				}
			}
		}
	}
}

group should be the easiest to implement, each group would be in a separate tab.
requires should also be easy enough.

options will probably be some sort of context menu. The idea is that each option's value will be a variable that can be referenced in env and command.

I think it should look somewhat like this:

|-----------------|
| HUD elements  > ||----------------|
| Another option  || (*) Default    |
|-----------------|| ( ) Full       |
                   |----------------|
                   | [x] devinfo    |
                   | [x] fps        |
                   | [x] frametimes |
                   | ...            |
                   | [ ] samplers   |
                   |----------------|

@Lucki
Copy link
Contributor Author

Lucki commented Jul 9, 2020

I somewhat have the feeling that a group and multiple options are doing both the same things. Take a look at the following two examples, aren't they doing the same?

Two combining into one group
{
	"id": "dxvk_hud",
	"group": "DXVK",
	"env": {
		"DXVK_HUD": "${options:hud}"
	},
	"options": {
		"hud": {
			"description": "HUD elements",
			"type": "list",
			"separator": ",",
			"values": {
				"devinfo": "Show name of GPU and driver version",
				"fps": "Show frame rate",
				"frametimes": "Show a frame time graph",
				"submissions": "Show number of command buffers submitted per frame",
				"drawcalls": "Show number of draw calls and render passes per frame",
				"pipelines": "Show total number of graphics and compute pipelines",
				"memory": "Show amount of device memory allocated and used",
				"gpuload": "Show estimated GPU load. May be inaccurate",
				"version": "Show DXVK version",
				"api": "Show D3D feature level used by the application",
				"compiler": "Show shader compiler activity",
				"samplers": "Show current number of sampler pairs used [D3D9 Only]"
			},
			"presets": {
				"default": {
					"name": "Default",
					"description": "Show name of GPU, driver version and frame rate",
					"value": "1"
				},
				"full": {
					"name": "Full",
					"description": "Show all available HUD elements",
					"value": "full"
				}
			}
		}
	}
}

{
	"id": "dxvk_log",
	"group": "DXVK",
	"env": {
		"DXVK_LOG_LEVEL": "${options:log}"
	},
	"options": {
		"log": {
			"description": "Log elements",
			"type": "one only",
			"values": {
				"none": "Log nothing",
				"error": "Log error messages",
				"warn": "Log warnings",
				"info": "Log info messages",
				"debug": "Log debug messages",
			}
		}
	}
}
One with multiple options
{
	"id": "dxvk",
	"env": {
		"DXVK_HUD": "${options:hud}",
		"DXVK_LOG_LEVEL": "${options:log}"
	},
	"options": {
		"hud": {
			"description": "HUD elements",
			"type": "list",
			"separator": ",",
			"values": {
				"devinfo": "Show name of GPU and driver version",
				"fps": "Show frame rate",
				"frametimes": "Show a frame time graph",
				"submissions": "Show number of command buffers submitted per frame",
				"drawcalls": "Show number of draw calls and render passes per frame",
				"pipelines": "Show total number of graphics and compute pipelines",
				"memory": "Show amount of device memory allocated and used",
				"gpuload": "Show estimated GPU load. May be inaccurate",
				"version": "Show DXVK version",
				"api": "Show D3D feature level used by the application",
				"compiler": "Show shader compiler activity",
				"samplers": "Show current number of sampler pairs used [D3D9 Only]"
			},
			"presets": {
				"default": {
					"name": "Default",
					"description": "Show name of GPU, driver version and frame rate",
					"value": "1"
				},
				"full": {
					"name": "Full",
					"description": "Show all available HUD elements",
					"value": "full"
				}
			}
		},
		"log": {
			"description": "Log elements",
			"type": "one only",
			"values": {
				"none": "Log nothing",
				"error": "Log error messages",
				"warn": "Log warnings",
				"info": "Log info messages",
				"debug": "Log debug messages",
			}
		}
	}
}

@tkashkin
Copy link
Owner

tkashkin commented Jul 9, 2020

ad13170 implements tweaks grouping.

image

Do you think it's good enough for now or is options idea worth implementing?

@Lucki
Copy link
Contributor Author

Lucki commented Jul 9, 2020

I guess it's fine as long as all options aren't excluding others. But I also don't have examples for this case other than log level which wouldn't really fit into this list anyway.
Guess it's ok for now.

Shall I rebase onto the refactoring branch? Maybe add some others, vkBasalt, Mumble, ideas?

@Lucki Lucki changed the base branch from dev to refactoring July 10, 2020 12:35
tkashkin added a commit that referenced this pull request Jul 11, 2020
Initial Tweak options UI (#355)
@tkashkin
Copy link
Owner

c16b87b implements executable and kernel module requirements for tweaks. It also implements initial UI for options:

tweak_options

@Lucki
Copy link
Contributor Author

Lucki commented Jul 11, 2020

Something visually:

Something logically:

  • Why is the reference ${option:hud} while the the setting is options:{hud:…}? I would try to name them identical to avoid confusion. option <-> options

Something practically:

  • The menu closes when selecting options which isn't practical when someone wants to select multiple options.
  • It should be possible to deselect presets. Maybe add a "custom" preset which allows to select the options further down?
  • The edit file button takes me to a read only file. Maybe copy the file to the first editable location? Not sure about this one because that will take precedence on updates.

@tkashkin
Copy link
Owner

GTK allows for popover and sliding panes, gedit makes use of these in their main menu - maybe that's an option here

Popover menu API is very weird and I can't find any good example on how to create checkboxes and radiobuttons dynamically in code without using .ui layouts.
Something custom would probably be the best here, but current menu should be enough for now at least for initial implementation and testing.

Why is the reference ${option:hud} while the the setting is options:{hud:…}? I would try to name them identical to avoid confusion. option <-> options

How about a different syntax for options? Variables are currently ${var}/$var.
Options can be something like @{optname} or #{optname} for example.

The menu closes when selecting options which isn't practical when someone wants to select multiple options.

Not sure if it's possible to solve for old menus without horrible hacks. Again, something custom is probably needed.

It should be possible to deselect presets.

Good point.

The edit file button takes me to a read only file. Maybe copy the file to the first editable location? Not sure about this one because that will take precedence on updates.

That's a good idea. I don't think updates will be a huge problem, bundled tweaks shouldn't change too much.

@Lucki
Copy link
Contributor Author

Lucki commented Jul 12, 2020

I can't find any good example on how to create checkboxes and radiobuttons dynamically in code without using .ui layouts.

Here are various examples.

These examples generate them without .ui layouts:

Popover menu API is very weird

AFAIK you can use any container inside a popover and it seems a gtk.menu…

[…] is just a container. Create a Container, like Gtk.Grid, pack a menu, and pack that parent container in the popover. Or better, just pack the menu directly.

tkashkin added a commit that referenced this pull request Jul 19, 2020
@tkashkin
Copy link
Owner

@Lucki what do you think about this? (c577bc1)

image

@Lucki
Copy link
Contributor Author

Lucki commented Jul 21, 2020

This looks pretty neat with the popover, it's a great visual upgrade.

I've recreated your test cases here and Test option 1 and 2 are looking good. The spacing makes sense between the options and presets/value pane and it's mostly possible to read everything.

Visual things:

  • For all three panes open at once it looks too narrow. You can't read either description of presets or values and the middle preset pane is mostly empty.
  • The hover highlight stays on the clicked item. I don't recall seeing that somewhere else.

I've also tried to give MangoHud some more options to test this. Sadly I wasn't that successful overall. Here are some issues I had:

  • Just turning options on isn't always enough.
    • Some options are on by default and need to be disabled by setting them to 0.
    • Some options require a number/text input, fps_limit as an example wants a number, media_player_name wants a text string.
  • I initially wanted to use "options" to group by logical things. GPU-related, CPU-related, appearance, and so on. But it doesn't seem to work that way because
    • All "options" are using the same environment variable. So I should set it to "MANGOHUD_CONFIG": "${option:gpu},${options:cpu},${option:appearance}"?
    • There's a preset full which would span over multiple "options".
  • Because the above I've only created one option which only displays two panes (preset and values) which doesn't look that good. Half of the popover is filled with empty space.
    Bildschirmfoto-20200721150431-952x695

While the things mentioned related to MangoHud are probably overkill (again, pls ignore if too much) the three paned layout isn't the best possible way to present the options overall IMHO. With only two options and two presets most of the space is empty.
I'm not sure how to solve this entirely, this is probably something for the UI design discussion again. Looking at the list of all available widgets:

  • Maybe it's better to stack presets on top of values again as you've thought initially in a ListBox (or similar). That will make sure there's no empty space below a low preset count.
    ListBox
  • For multiple options this seems more difficult. Things I can imagine being possible:
    • StackSidebar, prone to empty space again!
      StackSidebar
    • Notebook, same as already used for grouping
      Notebook
    • Don't always show all options but let the user click through the different options like my previous gedit example. This would make sure there's only one list at a time visible:
      • List of options the user can choose from
      • List of presets and values to choose from
      • I think they're using a Revealer to make the transition.

@tkashkin
Copy link
Owner

tkashkin commented Jul 31, 2020

I think that should be good enough for now (3dbccd3). Some empty space in options list is ok.

image

Don't always show all options but let the user click through the different options like my previous gedit example.

This will increase number of clicks and it may be harder to understand if presets and values are in additional submenus.

Just turning options on isn't always enough.
Some options are on by default and need to be disabled by setting them to 0.
Some options require a number/text input, fps_limit as an example wants a number, media_player_name wants a text string.

Presets should probably be enough for most cases, but maybe I'll add support for string options later.
Options already have a type property, currently list is the only supported type.

All "options" are using the same environment variable. So I should set it to "MANGOHUD_CONFIG": "${option:gpu},${options:cpu},${option:appearance}"?

I think so, but it may not work correctly if program (MangoHud in this example) doesn't handle weird cases like multiple consecutive separators (,,,option1,option2,,optionN).

@tkashkin
Copy link
Owner

tkashkin commented Aug 1, 2020

string options are supported now:

image

property
type string
value option value. ${value}/$value will be replaced with entered text
Test option code
"fps_limit": {
	"name": "FPS limit",
	"type": "string",
	"value": "fps_limit=${value}",
	"presets": {
		"240": {
			"name": "240 FPS",
			"value": "fps_limit=240"
		},
		"144": {
			"name": "144 FPS",
			"value": "fps_limit=144"
		},
		"120": {
			"name": "120 FPS",
			"value": "fps_limit=120"
		},
		"60": {
			"name": "60 FPS",
			"value": "fps_limit=60"
		},
		"30": {
			"name": "30 FPS",
			"value": "fps_limit=30"
		}
	}
}

@Lucki
Copy link
Contributor Author

Lucki commented Aug 2, 2020

That's great, I guess everything should now be possible input-wise. While adding my stuff I often struggled if I should add the options separately grouped or as a single slider with multiple options. Do you advise one over the other? Should I combine all proton settings into one slider, as in libstrangle? Everything combined seemingly has the advantage of a sorting which corresponds to the underlaying json.

Things to note:

  • Too many/long groups scrolls the entire window instead of only the tabs in the group header
  • I've noticed that the uppermost preset is active by default so I adapted to that scheme with my additions.
  • It would probably be useful to sort unavailable list entries to the bottom.

@tkashkin
Copy link
Owner

tkashkin commented Aug 2, 2020

While adding my stuff I often struggled if I should add the options separately grouped or as a single slider with multiple options.

I think separate tweaks are better if they are a single binary state (on/off). I've changed some NVIDIA options into separate tweaks because I think it should be easier to understand.

Too many/long groups scrolls the entire window instead of only the tabs in the group header

Fixed. I think we should try to reduce the number of groups though.
Maybe we should even get rid of less useful tweaks and possibly move them to the Wiki or separate repo.

It would probably be useful to sort unavailable list entries to the bottom.

Tweaks are sorted now.

@Lucki
Copy link
Contributor Author

Lucki commented Aug 3, 2020

I think we should try to reduce the number of groups though.

We can move the dxvk hud to the other overlays.

I've changed some NVIDIA options into separate tweaks because I think it should be easier to understand.

Well, the vblank option is tied to gsync so currently the meaning is a bit off. Maybe we should scrap both gsync options as it's likely already set in the nvidia-settings.

Maybe we should even get rid of less useful tweaks and possibly move them to the Wiki or separate repo.

Agreed. Maybe just a different branch with only tweaks is already enough. Personally I'm not a big fan of putting these things in the wiki because in a repo or branch it's much easier to get and update the files. (no manual copying required)

Also I'm sorry, I did a mistake in the winedlloverrides. winex11.drv=d only works in cli applications and even prevent graphical applications from running. The proper override should be mscoree=d;mshtml=d but that only takes care of the wine mono and gecko install questions, the prefix update dialog still appears with them.

@tkashkin
Copy link
Owner

tkashkin commented Aug 3, 2020

I don't think that Wine tweak is even needed since overrides are already set in code: https://github.com/tkashkin/GameHub/blob/refactoring/src/data/compat/Wine.vala#L259

@Lucki
Copy link
Contributor Author

Lucki commented Aug 3, 2020

I don't think that Wine tweak is even needed since overrides are already set in code: https://github.com/tkashkin/GameHub/blob/refactoring/src/data/compat/Wine.vala#L259

Then again, sorry, I wasn't aware of that (or forgot).

tkashkin added a commit that referenced this pull request Aug 3, 2020
Remove some of the less useful tweaks (#355)
tkashkin added a commit that referenced this pull request Aug 3, 2020
TweaksList: sort tweak groups by name
tkashkin added a commit that referenced this pull request Aug 10, 2020
Tweak options: saving/restoring
Tweak states (`enabled`/`disabled`/`global`)
@tkashkin
Copy link
Owner

86e59c3 implements saving and restoring tweak options.

Also tweak state for games is enabled/disabled/global now, that should fix #326 (comment).

@tkashkin
Copy link
Owner

86788a0 reenables some of the functionality that was disabled due to refactoring.

Now refactoring branch can actually save, install and run some games (only native for now), tweaks also seem to work.

Remove old gamehub.db before testing because executable path format was changed and there's currently no way to update executable path for game in UI.

I'm not sure how to handle tweak options when tweak state for game is global, which way is better?

  • Ignore options set for the game and use global options
  • Use game's options, copy them from global if they weren't set for the game
  • Somehow merge them

@Lucki
Copy link
Contributor Author

Lucki commented Aug 11, 2020

how to handle tweak options when tweak state for game is global

I actually had to rethink this multiple times.

Looking closely a game tweak can have four states:

  • default on (global) (gray representing the global state)
  • default off (global) (gray representing the global state)
  • force on (white)
  • force off (white)
  • currently there is no way to go out of the forced on/off back to the default

And while having these states for tweaks they would also exist for every option inside a tweak.

  • Default off and forced off is just off.
  • If you want to allow overriding options while the tweak is in a default on state you have to override them - everything is default unless an option is manually forced on/off.
  • Easier would be to disallow ambiguous options by disabling the gear button until the tweak is forced on. In this case I probably would completely ignore the default options.

So IMHO it depends on what you want, support ambiguous options or not.

Did I answer the question? If not: please elaborate a bit more.

Notes:

  • The gray for default looks a bit close to disabled, could be my theme though
  • The game tweak window is tiny

@tkashkin
Copy link
Owner

tkashkin commented Aug 11, 2020

So this is probably the most intuitive and the easiest to implement solution:

  • global enabled - use global options
  • global disabled - do nothing
  • enabled - use game options, copy global options if game options are not set
  • disabled - do nothing

The gray for default looks a bit close to disabled, could be my theme though

I don't really know how to show global states, there's a tooltip, but it would be nice if these states were somehow more visible. Any ideas?

@Lucki
Copy link
Contributor Author

Lucki commented Aug 12, 2020

How about a simple checkbox or "toggle button" to toggle the global state - they clearly look pressed or ticked on/off and also provide a way to reset without an additional button.

Other than that it has to be some visual indicator. Lutris does this by making the setting bold and providing a button to reset which also isn't that intuitive. Other possibilities are underline or italic as well as a different text or background color.

tkashkin added a commit that referenced this pull request Aug 12, 2020
…m global (#355)

GamePropertiesDialog: Executable tab
@tkashkin
Copy link
Owner

How about a simple checkbox or "toggle button" to toggle the global state - they clearly look pressed or ticked on/off and also provide a way to reset without an additional button.

I don't really see how it should look and where to put it. I'm also not sure if "Reset to global" functionality is really needed, it will just complicate UI even more imo.

Other than that it has to be some visual indicator

The only solution I can think of is to show tweak state as text near tweak title or description like additional info is already shown in installers list in game installation dialog.

@Lucki
Copy link
Contributor Author

Lucki commented Aug 12, 2020

I'm also not sure if "Reset to global" functionality is really needed

Without that it would never be possible to go out of the special state and let the game tweaks represent the global state again. May it because accidental misclicking or changing their mind. Maybe making one reset button / switch for all game tweaks is worth a thought. Basically activating or deactivating all game tweaks at once bringing them into their forced on/off state based on the global default or being able to reset them all at once.

as text near tweak title or description

Also not too bad, that would at least make immediately clear what it's representing. But would be bad if it's always hidden in the tooltip because the descriptions are too long.

  • The game tweak window is tiny

Sry, scrap that, I looked at the old window.

@tkashkin
Copy link
Owner

Now there's a button to reset global or game tweak options. Tweak state is shown as text.

image

@Lucki
Copy link
Contributor Author

Lucki commented Aug 13, 2020

Looks good and fitting and I like that you can also find that button on the global tweak list :)
I'm also happy with the text describing why something is enabled.

Not sure how ready it is: Switching the game tweak or option also switches the global tweak or option.
An idea for too long descriptions would be to expand the list row to the bottom on a click on the row or an expand arrow to fit the whole description instead of using a tooltip.

tkashkin added a commit that referenced this pull request Aug 15, 2020
GameInstallDialog/InstallTask: support install_dir import
@tkashkin
Copy link
Owner

Not sure how ready it is: Switching the game tweak or option also switches the global tweak or option

748c4f7 fixes some bugs for tweak options, was that it or did I miss something else?

An idea for too long descriptions would be to expand the list row to the bottom on a click on the row or an expand arrow to fit the whole description instead of using a tooltip.

I don't like this idea. Start of the description is usually enough to understand it, full description is shown in the tooltip if it's needed.
I think it looks much worse if rows don't have consistent heights:

image

@Lucki
Copy link
Contributor Author

Lucki commented Aug 15, 2020

was that it or did I miss something else?

It's still the same with 0.16.0-d2ec729? https://i.imgur.com/X5pzNTW.mp4 (dunno where that glitching comes from, pls ignore)
Haven't found something else.

I don't like this idea.

That's totally fine.

Maybe we should even get rid of less useful tweaks and possibly move them to the Wiki or separate repo.

Agreed. Maybe just a different branch with only tweaks is already enough. Personally I'm not a big fan of putting these things in the wiki because in a repo or branch it's much easier to get and update the files. (no manual copying required)

Did you decide on this? I would like to try making the bundled tweaks more complete in the branch/repo/wiki if that's something you'd like to have.

tkashkin added a commit that referenced this pull request Aug 15, 2020
@tkashkin
Copy link
Owner

It's still the same with 0.16.0-d2ec729

Should be fixed by 9458fb8.

Did you decide on this?

You can add new tweaks to this PR for now and I'll think where to put them later.
I'll probably bundle the most useful ones and move others to a separate branch.

@Lucki
Copy link
Contributor Author

Lucki commented Oct 26, 2020

Well, that was a mistake…
Github didn't liked that independent branch and auto-closed this with no option for me to reopen.

Let me know if I should create a new PR. https://github.com/Lucki/GameHub/tree/more_tweaks

tkashkin added a commit that referenced this pull request Sep 1, 2021
Initial Tweak options UI (#355)


Former-commit-id: c16b87b
tkashkin added a commit that referenced this pull request Sep 1, 2021
tkashkin added a commit that referenced this pull request Sep 1, 2021
tkashkin added a commit that referenced this pull request Sep 1, 2021
Remove some of the less useful tweaks (#355)


Former-commit-id: 18d8f38
tkashkin added a commit that referenced this pull request Sep 1, 2021
TweaksList: sort tweak groups by name

Former-commit-id: f590902
tkashkin added a commit that referenced this pull request Sep 1, 2021
Tweak options: saving/restoring
Tweak states (`enabled`/`disabled`/`global`)

Former-commit-id: 86e59c3
tkashkin added a commit that referenced this pull request Sep 1, 2021
…m global (#355)

GamePropertiesDialog: Executable tab

Former-commit-id: a97ce7e
tkashkin added a commit that referenced this pull request Sep 1, 2021
tkashkin added a commit that referenced this pull request Sep 1, 2021
GameInstallDialog/InstallTask: support install_dir import

Former-commit-id: 748c4f7
tkashkin added a commit that referenced this pull request Sep 1, 2021
…serializing and deserializing them (#355)

Former-commit-id: 9458fb8
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.

2 participants