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

Scale unity GUI #1152

Merged
merged 18 commits into from
Sep 7, 2021
Merged

Scale unity GUI #1152

merged 18 commits into from
Sep 7, 2021

Conversation

kianzarrin
Copy link
Collaborator

@kianzarrin kianzarrin commented Sep 3, 2021

fixes #573
Speed limit tool and TTL now have constant size regardless of resolution (+ minor bug fixes)

To test this you need to remove TMPE_GlobalConfig.xml or set GuiScaleToResolution to true.
we have not released this so it won't be an issue for WS users but what about testers?

TMPE.zip

@kianzarrin kianzarrin changed the base branch from master to log-dir September 3, 2021 00:05
@kianzarrin kianzarrin changed the base branch from log-dir to master September 3, 2021 00:07
@kianzarrin kianzarrin changed the base branch from master to log-dir September 3, 2021 00:07
@kianzarrin kianzarrin self-assigned this Sep 3, 2021
@kianzarrin kianzarrin added UI User interface updates SPEED LIMITS Feature: Speed limits TRAFFIC LIGHTS Feature: Traffic lights - toggle, timed, etc labels Sep 3, 2021
@krzychu124
Copy link
Member

To test this you need to remove TMPE_GlobalConfig.xml or set GuiScaleToResolution to true.

Just bump up GlobalConfig version and mod will recreate it with new settings

we have not released this so it won't be an issue for WS users but what about testers?

I don't understand your concern 🤔

Anyways I'll test it later today 😉

@DaEgi01 DaEgi01 self-requested a review September 3, 2021 17:22
@@ -49,14 +49,14 @@ public static class UIScaler {
public static float MaxWidth {
get {
float ret = Config.GuiScaleToResolution ? BaseResolutionX : Screen.width;
return ret / Config.GuiScale;
return ret / (Config.GuiScale * 0.01f);
Copy link
Contributor

Choose a reason for hiding this comment

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

no magic numbers plz. make the 0.01f a variable.

Copy link
Member

Choose a reason for hiding this comment

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

@kianzarrin I don't remember what range GuiScale is (probably 30...200+) but if it's percentage then it does not look right to me. For 100 it will return ret/1f which is ok, but for 200 it will be ret/2f, so 2x smaller?? 🤔

}
}

public static float MaxHeight {
get {
float ret = Config.GuiScaleToResolution ? BaseResolutionY : Screen.height;
return ret / Config.GuiScale;
return ret / (Config.GuiScale * 0.01f);
Copy link
Contributor

Choose a reason for hiding this comment

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

same var

@@ -142,6 +143,8 @@ public SpeedLimitsTool(TrafficManagerTool mainTool)

public override void OnToolGUI(Event e) {
base.OnToolGUI(e);
var oldMatrix = GUI.matrix;
GUI.matrix = UIScaler.ScaleMatrix;
Copy link
Contributor

@DaEgi01 DaEgi01 Sep 3, 2021

Choose a reason for hiding this comment

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

what is this matrix switching thing?
currently on the train, can't look it up easily.
is GUI.matrix our code?
if not and we can't change it than this is ok.
if its ours we really should change it :)

Copy link
Member

Choose a reason for hiding this comment

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

GUI.matrix is Unity code for immediate gui, works pretty much like GUI.color, but obviously changes different thing 😉

Copy link
Contributor

Choose a reason for hiding this comment

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

kk .. in that case the code is fine.

@@ -308,6 +309,8 @@ public TimedTrafficLightsTool(TrafficManagerTool mainTool)

public override void OnToolGUI(Event e) {
base.OnToolGUI(e);
var oldMatrix = GUI.matrix;
GUI.matrix = UIScaler.ScaleMatrix;
Copy link
Contributor

Choose a reason for hiding this comment

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

same as for speedlimitstool.

Copy link
Contributor

Choose a reason for hiding this comment

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

all fine

Base automatically changed from log-dir to master September 3, 2021 22:12
@DaEgi01 DaEgi01 self-requested a review September 3, 2021 22:17
File.Delete(LogFilePath); // delete old file to avoid confusion.
}

var args = Environment.GetCommandLineArgs().ToList();
Copy link
Contributor

Choose a reason for hiding this comment

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

No need to convert ToList() here since you don't use it as a list further down in the method.
Better to keep it a string array and use Array.FindIndex(args, a => a == "-logFile") in the next line.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

this is not part of this review. I updated the branch

@kianzarrin kianzarrin requested a review from DaEgi01 September 4, 2021 21:56
Copy link
Member

@krzychu124 krzychu124 left a comment

Choose a reason for hiding this comment

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

TTL signs gui is correctly positioned, all good 👍

@kianzarrin kianzarrin merged commit 3871609 into master Sep 7, 2021
@kianzarrin kianzarrin deleted the scale-Unity-GUI branch September 7, 2021 18:21
@originalfoo originalfoo added this to the 11.6.0 milestone Dec 10, 2021
@originalfoo originalfoo modified the milestones: 11.6.0, 11.6.1.2 Jan 18, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
SPEED LIMITS Feature: Speed limits TRAFFIC LIGHTS Feature: Traffic lights - toggle, timed, etc UI User interface updates
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Speed limits window too constrained on QHD resolution
5 participants