/openage

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

[WIP] Settings menu placeholder #749

Open
wants to merge 24 commits into
base: master
from
+1,215 −30

Conversation

Labels
c++ improvement nice new thing ☺ ui
4 participants
@ChipmunkV @lisacvuk @heinezen @TheJJ
@ChipmunkV
Copy link
Contributor

ChipmunkV commented Feb 18, 2017 •
edited

#673
#728
#732
#751

  • SettingsMode and its TabView with global font size setting in "Graphics" tab
  • Global font size cvar
  • Write back cvars to config file when calling set
  • Initial propagation of the cvar list to gui
  • on-changed propagation from gui to cvar
  • on-cvar-changed callbacks (that will allow on-changed propagation from cvar to gui)
  • caching of the cvars that are loaded but not yet taken by any subsystem of the engine
  • Batch write back to config file (instead on writing each time). Ok, write them one by one, but in different thread

Random unrelated stuff:

  • Keep mode when reloading gui
  • Fix input crash on reload

@ChipmunkV ChipmunkV force-pushed the ChipmunkV:settings branch from a30eb94 to f41935a Feb 18, 2017

@TheJJ TheJJ added c++ nice new thing ☺ ui improvement labels Feb 19, 2017

@ChipmunkV ChipmunkV force-pushed the ChipmunkV:settings branch 2 times, most recently from 61f4640 to c8ea3be Feb 19, 2017

@lisacvuk

This comment has been minimized.

Sign in to view
Copy link

lisacvuk commented Feb 24, 2017 •
edited

Once compiled, how do I test it? Thanks!
EDIT: Got it! It's 'Change Mode' everyone!
@lisacvuk

This comment has been minimized.

Sign in to view
Copy link

lisacvuk commented Feb 24, 2017

Screenshots:
http://i.imgur.com/LK912Ey.png
http://i.imgur.com/oNTtv6E.png
The rest of the tabs are empty.
@heinezen

This comment has been minimized.

Sign in to view
Copy link
Contributor

heinezen commented Feb 24, 2017

@lisacvuk Testing of the whole UI happens in #751 ;)

@ChipmunkV ChipmunkV force-pushed the ChipmunkV:settings branch 3 times, most recently from 75dad3e to 4b87541 Feb 25, 2017

@ChipmunkV ChipmunkV changed the title [WIP] Settings menu placeholder Settings menu placeholder Feb 25, 2017

@TheJJ

TheJJ reviewed Feb 25, 2017
View changes

libopenage/cvar/cvar.cpp Outdated
}();

if (setter) {
setter(value);

This comment has been minimized.

Sign in to view
Copy link
@TheJJ

TheJJ Feb 25, 2017

Member

can you simplify that construct by just finding the store element? I missed the () and thought you'd check if (lambdafunction), which made no sense. If it's for the lock, you can either unlock it manually afterwards or put it in a separate scope.
libopenage/cvar/cvar.cpp Outdated
auto writeback_shared = this->writeback;

this->job_manager->enqueue<int>([writeback_shared] {
std::lock_guard<std::mutex> lock(writeback_shared->elements_mutex);

This comment has been minimized.

Sign in to view
Copy link
@TheJJ

TheJJ Feb 25, 2017

Member

why does the setting need to be in a different thread?

This comment has been minimized.

Sign in to view
Copy link
@ChipmunkV

ChipmunkV Feb 25, 2017

Contributor

Felt slow when was changing a parameter by scrolling the mousewheel.

@ChipmunkV ChipmunkV force-pushed the ChipmunkV:settings branch 4 times, most recently from 4b2ae5d to 17b6d90 Feb 25, 2017

@heinezen

This comment has been minimized.

Sign in to view
Copy link
Contributor

heinezen commented Mar 4, 2017

OK, finally had a look at it myself :)

  1. Instead of anchoring the tabs in the middle of the top, an anchor point further to the left would be nice. After all, this is were most players will look first.
  2. Seeing how Font Size also affects UI scale, shouldn't we just call the option "UI Scale"? Furthermore the selection of a font size is very finicky and requires readjusting the mouse pointer every time. We either should have presets (like I did it in the mockup) or a UI Scale slider with percentage numbers.
@TheJJ

This comment has been minimized.

Sign in to view
Copy link
Member

TheJJ commented Mar 8, 2017

I changed the cvar manager a bit in #723, how should we proceed? Which one should go first? :)
@ChipmunkV

This comment has been minimized.

Sign in to view
Copy link
Contributor

ChipmunkV commented Mar 8, 2017

@TheJJ at a first glance, #723 doesn't deal with file contents. So it'll be easy.

I've got time to merge settings later.
@TheJJ

This comment has been minimized.

Sign in to view
Copy link
Member

TheJJ commented Mar 21, 2017

cvar stuff is merged, you may adapt this now please.
@TheJJ

This comment has been minimized.

Sign in to view
Copy link
Member

TheJJ commented Apr 22, 2017

Ping? :)

@ChipmunkV ChipmunkV force-pushed the ChipmunkV:settings branch from 17b6d90 to cd62d2f Apr 29, 2017

@ChipmunkV

This comment has been minimized.

Sign in to view
Copy link
Contributor

ChipmunkV commented Apr 29, 2017

Two things:

  • can't figure out into what file to write the changes (so hardcoded the keybinds.oac)
  • can't write the changes
@TheJJ

This comment has been minimized.

Sign in to view
Copy link
Member

TheJJ commented Apr 30, 2017

For those two things, I just created #813 :)
@ChipmunkV

This comment has been minimized.

Sign in to view
Copy link
Contributor

ChipmunkV commented May 1, 2017

For the second one it's about inability to edit files with the new path system. I still need to figure out why it doesn't accept open("a+").

@ChipmunkV ChipmunkV referenced this pull request May 1, 2017

Open

[WIP] speed up initial loading of the textures #810

@TheJJ

This comment has been minimized.

Sign in to view
Copy link
Member

TheJJ commented May 13, 2017

Any luck finding out why a+ doesn't work? Does w work?

@TheJJ TheJJ referenced this pull request Apr 20, 2018

Merged

fslike: add readwrite and append modes for opening files #996

@TheJJ

This comment has been minimized.

Sign in to view
Copy link
Member

TheJJ commented Apr 23, 2018

I implemented file append modes in #996.
I'm not sure though how we should edit our config files properly, because it's quite tricky to a) allow users to write the files as they like and b) update just the values at the right place automatically. Basically we'd have to parse the files and then rewrite them retaining all information including comments and whitespace, etc.
@ChipmunkV

This comment has been minimized.

Sign in to view
Copy link
Contributor

ChipmunkV commented Apr 24, 2018

Basically we'd have to parse the files and then rewrite them retaining all information including comments and whitespace, etc.

That's roughly what this PR does. But it deletes the comment on the line where an option is overwritten (probably not the worst thing to do when I think about it).
@TheJJ

This comment has been minimized.

Sign in to view
Copy link
Member

TheJJ commented Apr 24, 2018

Ah okay, I didn't realize that since you asked for the append functionality.
Is something else missing or is this nearly reviewable/mergeable?

@TheJJ TheJJ added this to in progress in user interface May 2, 2018

@zuntrax zuntrax changed the title Settings menu placeholder [WIP] Settings menu placeholder Jan 27, 2019

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment