Details
- Reviewers
elexis - Group Reviewers
Restricted Owners Package (Owns No Changed Paths) Restricted Owners Package (Owns No Changed Paths)
- Run the game
- Open options and check buttons
Diff Detail
- Repository
- rP 0 A.D. Public Repository
- Lint
Lint Skipped - Unit
Unit Tests Skipped - Build Status
Buildable 3639 Build 6315: Vulcan Build (Windows) Jenkins Build 6314: Vulcan Build Jenkins
Event Timeline
Successful build - Chance fights ever on the side of the prudent.
Updating workspaces...
FCollada/FCDocument/FCDLibrary.cpp: In instantiation of ‘void LibraryExport() [with T = FCDAnimation]’:
FCollada/FCDocument/FCDLibrary.cpp:149:30: required from here
FCollada/FCDocument/FCDLibrary.cpp:141:11: warning: variable ‘cptr’ set but not used [-Wunused-but-set-variable]
const T* cptr = ((const FCDLibrary<T>*)l1)->GetEntity(0);
^
FCollada/FCDocument/FCDLibrary.cpp: In instantiation of ‘void LibraryExport() [with T = FCDAnimationClip]’:
FCollada/FCDocument/FCDLibrary.cpp:150:34: required from here
FCollada/FCDocument/FCDLibrary.cpp:141:11: warning: variable ‘cptr’ set but not used [-Wunused-but-set-variable]
FCollada/FCDocument/FCDLibrary.cpp: In instantiation of ‘void LibraryExport() [with T = FCDCamera]’:
FCollada/FCDocument/FCDLibrary.cpp:151:27: required from here
FCollada/FCDocument/FCDLibrary.cpp:141:11: warning: variable ‘cptr’ set but not used [-Wunused-but-set-variable]
FCollada/FCDocument/FCDLibrary.cpp: In instantiation of ‘void LibraryExport() [with T = FCDController]’:
FCollada/FCDocument/FCDLibrary.cpp:152:31: required from here
FCollada/FCDocument/FCDLibrary.cpp:141:11: warning: variable ‘cptr’ set but not used [-Wunused-but-set-variable]
FCollada/FCDocument/FCDLibrary.cpp: In instantiation of ‘void LibraryExport() [with T = FCDEffect]’:
FCollada/FCDocument/FCDLibrary.cpp:153:27: required from here
FCollada/FCDocument/FCDLibrary.cpp:141:11: warning: variable ‘cptr’ set but not used [-Wunused-but-set-variable]
FCollada/FCDocument/FCDLibrary.cpp: In instantiation of ‘void LibraryExport() [with T = FCDEmitter]’:
FCollada/FCDocument/FCDLibrary.cpp:154:28: required from here
FCollada/FCDocument/FCDLibrary.cpp:141:11: warning: variable ‘cptr’ set but not used [-Wunused-but-set-variable]
FCollada/FCDocument/FCDLibrary.cpp: In instantiation of ‘void LibraryExport() [with T = FCDForceField]’:
FCollada/FCDocument/FCDLibrary.cpp:155:31: required from here
FCollada/FCDocument/FCDLibrary.cpp:141:11: warning: variable ‘cptr’ set but not used [-Wunused-but-set-variable]
FCollada/FCDocument/FCDLibrary.cpp: In instantiation of ‘void LibraryExport() [with T = FCDGeometry]’:
FCollada/FCDocument/FCDLibrary.cpp:156:29: required from here
FCollada/FCDocument/FCDLibrary.cpp:141:11: warning: variable ‘cptr’ set but not used [-Wunused-but-set-variable]
FCollada/FCDocument/FCDLibrary.cpp: In instantiation of ‘void LibraryExport() [with T = FCDImage]’:
FCollada/FCDocument/FCDLibrary.cpp:157:26: required from here
FCollada/FCDocument/FCDLibrary.cpp:141:11: warning: variable ‘cptr’ set but not used [-Wunused-but-set-variable]
FCollada/FCDocument/FCDLibrary.cpp: In instantiation of ‘void LibraryExport() [with T = FCDLight]’:
FCollada/FCDocument/FCDLibrary.cpp:158:26: required from here
FCollada/FCDocument/FCDLibrary.cpp:141:11: warning: variable ‘cptr’ set but not used [-Wunused-but-set-variable]
FCollada/FCDocument/FCDLibrary.cpp: In instantiation of ‘void LibraryExport() [with T = FCDMaterial]’:
FCollada/FCDocument/FCDLibrary.cpp:159:29: required from here
FCollada/FCDocument/FCDLibrary.cpp:141:11: warning: variable ‘cptr’ set but not used [-Wunused-but-set-variable]
FCollada/FCDocument/FCDLibrary.cpp: In instantiation of ‘void LibraryExport() [with T = FCDSceneNode]’:
FCollada/FCDocument/FCDLibrary.cpp:160:30: required from here
FCollada/FCDocument/FCDLibrary.cpp:141:11: warning: variable ‘cptr’ set but not used [-Wunused-but-set-variable]
FCollada/FCDocument/FCDLibrary.cpp: In instantiation of ‘void LibraryExport() [with T = FCDPhysicsModel]’:
FCollada/FCDocument/FCDLibrary.cpp:161:33: required from here
FCollada/FCDocument/FCDLibrary.cpp:141:11: warning: variable ‘cptr’ set but not used [-Wunused-but-set-variable]
FCollada/FCDocument/FCDLibrary.cpp: In instantiation of ‘void LibraryExport() [with T = FCDPhysicsMaterial]’:
FCollada/FCDocument/FCDLibrary.cpp:162:36: required from here
FCollada/FCDocument/FCDLibrary.cpp:141:11: warning: variable ‘cptr’ sSomeone has a hypothetical issue related to this one. A hypothetical new tab in the options page could, hypothetically, have these new buttons. Should those be grouped equally?
Someone has a hypothetical issue related to this one. A hypothetical new tab in the options page could, hypothetically, have these new buttons. Should those be grouped equally?
Totally forgot about that :D
Edit: My answer would be No, as grouping makes it easier to differentiate the different button types.
Could, but shouldn't, many buttons look weird, but small number of buttons or buttons which couldn't be split on different types could be grouped. Also buttons should be aligned alone a grid of other elements, but not randomly.
The crux being that the close, save, reset (possibly revert) buttons are shared for all option page tabs (including the hypothetical new tab).
So it would look weird if they have different locations depending on the selected tab.
Do you suggest to use full width of the option page?
In common case I want to prevent empty spaces between buttons, which not aligned to any element grid.
Successful build - Chance fights ever on the side of the prudent.
Updating workspaces... Build (release)... Build (debug)... Running release tests... Running cxxtest tests (307 tests)...................................................................................................................................................................................................................................................................................................................OK! Running debug tests... Running cxxtest tests (307 tests)...................................................................................................................................................................................................................................................................................................................OK! Checking XML files...
How would using full width prevent different positions for the buttons depending on the selected tab in case of adding the new tab with more buttons?
(It can be considered irrelevant for now as there is no such tab, but then I might have to revert this addition.)
I think, tab specific buttons should be inside the tab, but not under it. Common buttons should be under the tab.
This is the current situation, compared to the applied D1017 and a version with a height increased by 2 pixels.
Should I update this diff, it is currently not working because of some changes in that file over the years.
I mean I'm not uninterested. Generally equally spaced looks better and @Langbart 's suggestion with D1017 + 2 Pixel height looks also better/ a bit less cramped.
The bigger problem, now that I think about it, is the inconsistency between the button positioning in all the different gui pages.
Mod Selection has the buttons snapped to the right and cancel is on the left side
Hotkeys uses the full width but has a hole in the middle and close on the right side.
Language -> full width and cancel on the left
Options -> subject to this patch but close on the right side
Imo:
The button to close a GUI page (cancel/close) should always be on the same position, either far left or far right (I think I like far right more)
Accordingly, Reset should be on the same position on the other end