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

gui: add commit hash to the game panel with non master builds #3993

Merged
merged 3 commits into from Jan 17, 2018
Merged

gui: add commit hash to the game panel with non master builds #3993

merged 3 commits into from Jan 17, 2018

Conversation

elad335
Copy link
Contributor

@elad335 elad335 commented Dec 27, 2017

No description provided.

@elad335 elad335 changed the title gui: add commit hash to the game panel gui: add commit hash to the game panel with non master builds Dec 27, 2017
@elad335
Copy link
Contributor Author

elad335 commented Dec 27, 2017

image

version = version + "-" + rpcs3::get_branch();
}
//Add branch and commit hash to version on frame , unless it's master.
version = is_master ? version.substr(0 , version.find_last_of("-")) : version.substr(0 , version.find_last_of(" ")) + " " + rpcs3::get_branch();
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this line is not indented correctly

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

👍

@scribam
Copy link
Contributor

scribam commented Jan 13, 2018

Nice addition, it would be certainly useful when people post gaming screenshots related to a PR.

Do you think we should have something more consistent between the gs window title (at the top) and the main window title (at the bottom)?

screenshot from 2018-01-13 16-58-38

@elad335
Copy link
Contributor Author

elad335 commented Jan 13, 2018

@scribam maximum the 'v' in the main window should be removed, that should do the trick

@scribam
Copy link
Contributor

scribam commented Jan 13, 2018

Or added in the gs window title 🙂 Doesn't matter which way 😄

What about the branch name? Do you think you can set it like this OpenGL | 0.0.4-6340-206f6a7 | pr-3993 instead of OpenGL | 0.0.4-6340-pr-3993 | 206f6a7 for the gs window (hash and branch name inverted)? We will be closer to the main window title, don't we?

@elad335
Copy link
Contributor Author

elad335 commented Jan 13, 2018

yeah 👍

Copy link
Contributor

@scribam scribam left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM 👍

screenshot from 2018-01-13 18-25-16

@AniLeo
Copy link
Member

AniLeo commented Jan 13, 2018

v in version should remain IMO, just in main window or in both windows

@AniLeo
Copy link
Member

AniLeo commented Jan 13, 2018

@elad335 Please remove commit count from non-master builds on version, there's a reason I removed it since 0.0.4, it's irrelevant since it's not a master build.

Just 0.0.4-d2a2a73 (3993) is fine , or like you have it right now but without commit count

@elad335
Copy link
Contributor Author

elad335 commented Jan 13, 2018

@AniLeo theres a bug that display the build count only on local builds ,ci builds dont display it
CI : VS
image
image
BTW hash on CI is the one that appveyor displays.

@AniLeo
Copy link
Member

AniLeo commented Jan 14, 2018

It's meant to display on local builds yeah, script only excludes if it's a Pull Request being built via AppVeyor.

Maybe add branch name together with version somehow in the same separator, otherwise title bar is starting to look polluted with too much informaton

@elad335
Copy link
Contributor Author

elad335 commented Jan 14, 2018

@AniLeo so remove the commit count from both versions and replace that with the branch name?

@AniLeo
Copy link
Member

AniLeo commented Jan 14, 2018

You can leave it as is locally

Copy link
Member

@AniLeo AniLeo left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@AniLeo AniLeo merged commit cc02ef6 into RPCS3:master Jan 17, 2018
@elad335 elad335 deleted the hash branch January 19, 2018 18:27
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants