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

Main menu and tool code refactoring #656

Merged
merged 23 commits into from
Feb 21, 2020

Conversation

kvakvs
Copy link
Collaborator

@kvakvs kvakvs commented Feb 6, 2020

What does

Affects Main TMPE button, and main TMPE panel

bild

  • Main panel and main button now scale to % of screen size
  • button states all reworked, normal sprited desaturated, active sprites full colour
  • textures split into sprites creating atlas on load
  • smart skinning per button, and buttons also control their state

More details

  • BaseUI and UIBase renamed to ModUI (class) and ModUi (field) representing root object of user interface for the TMPE mod;
  • Some generic UI code is now slowly moving out to TrafficManager.U namespace (BaseUButton from old LinearSpriteButton)
  • WIP on BaseUButton and ButtonTexture going to generalize textures handling from UIMainMenuButton
  • Get-only properties on BaseUButton renamed to Get* functions instead of just nouns.
  • New texture loading code for atlases is used for UI buttons, removing some loading code from the old static texture classes.
  • Renaming and documenting stuff.
  • Logic is mostly unchanged, but file structure can be changing, files are moved or split.

This is code cleanup and preparation for new main menu panel design.

@originalfoo originalfoo added code cleanup Refactor code, remove old code, improve maintainability technical Tasks that need to be performed in order to improve quality and maintainability UI User interface updates labels Feb 6, 2020
playAudioEvents = true;
}

public abstract bool Active { get; }
Copy link
Collaborator

Choose a reason for hiding this comment

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

Copy link
Collaborator Author

@kvakvs kvakvs Feb 6, 2020

Choose a reason for hiding this comment

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

Yeah this entire thing is convoluted, i want a more generic button with all these states, now also including the disabled state. Will have remember to take your change in too, whether your or mine gets merged first.

@originalfoo
Copy link
Member

Another state for buttons that might be useful is some sort of "highlight" effect?

Currently there's no visual feedback to users as to what bulk applicator tools are applying; usually it is the currently selected tool + one or more other tools.

For example, with priority signs tool selected, I can Ctrl+Shift+Click to apply priority signs, junction restrictions and lane arrows down a route. Would be nice if the toolbar buttons of those tools could light a different colour (eg. light blue bg to match the selection shape on map) letting me know I'm about to apply multiple tools.

At some point in future, it would also be nice to toggle persistent overlays by right-clicking buttons so maybe also an "overlay" state?

@kvakvs
Copy link
Collaborator Author

kvakvs commented Feb 7, 2020

I like the ideas but not going to do any of that until a clear state diagram is created.
If a possibility of state toggles like those right click overlays is considered, we can add that later to the design and amend the button features. But let's begin with just refactoring + changing textures for the main menu.

@originalfoo originalfoo added this to the 11.2 milestone Feb 7, 2020
@kvakvs kvakvs force-pushed the 523-facelift-mainmenu branch 2 times, most recently from 59ccc24 to aae9659 Compare February 17, 2020 23:53
Comment on lines 193 to 197
const int numRows = 2;
const int numCols = 6;
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Rework, move constants

TLM/TLM/UI/ModUI.cs Outdated Show resolved Hide resolved
@kvakvs kvakvs marked this pull request as ready for review February 18, 2020 21:27
@kianzarrin
Copy link
Collaborator


The number 11 initially made me think there are 11 new things to look at. like this:

maybe put V11 if it fits?

@originalfoo
Copy link
Member

The 11 is already in title bar of the TMPE menu bar. So I think it can be removed from the menu button.

@kvakvs
Copy link
Collaborator Author

kvakvs commented Feb 20, 2020

11 is small and easy to remove. Reason it is here, because i wanted the button to look slightly different from pre-11, to spark interest in those who still run v10 and didn't pay attention that TMPE has been upgraded to 11. One idea would be to take whatever we have for logo? The chirper and traffic light? And take away the crown. And another idea was to add version to it.

@@ -49,11 +52,11 @@ public class RemoveCitizenInstanceButtonExtender : MonoBehaviour {
return button;
}

public class RemoveCitizenInstanceButton : LinearSpriteButton {
public class RemoveCitizenInstanceButton : BaseUButton {
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Double check this still works and the icon is visible.

@@ -57,11 +60,11 @@ var publicTransportVehicleInfoPanel
return button;
}

public class RemoveVehicleButton : LinearSpriteButton {
public class RemoveVehicleButton : BaseUButton {
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Double check this works too, and the button is visible

@krzychu124
Copy link
Member

Scale slider works as intended, saves position. looks good 👍

@kvakvs
Copy link
Collaborator Author

kvakvs commented Feb 21, 2020

Now with maintenance features disabled, the menu is refreshed live and buttons are removed. As soon as a row is empty, the panel also shrinks.
bild

@kvakvs
Copy link
Collaborator Author

kvakvs commented Feb 21, 2020

Updated now:

  • Code (toolbutton atlases now use HashSet<string> instead of List<string> to fight duplicate sprite names)
  • Buttons updated in realtime as Maintenance tab has its features toggled
  • Added v11 to the main button sprite, and toned down the crown in the background.

@originalfoo
Copy link
Member

In DEBUG builds, ModUI.cs:

image

The name uiView does not exist in the current context

@kvakvs
Copy link
Collaborator Author

kvakvs commented Feb 21, 2020

@aubergine10
Fixed, i've commented too much last time
Removing code feels good.

@originalfoo
Copy link
Member

Tested out menu resizing when tools are disabled...

image

image

image

Is it possible to tweak so it works something like:

// pseudocode
if (numButtons > 4) {
   // arrange buttons on to two rows
   // but retain at least 4 buttons on top row
} else {
   // just one row with 4 or less buttons
}

@kvakvs
Copy link
Collaborator Author

kvakvs commented Feb 21, 2020

Possible.

@originalfoo
Copy link
Member

I think with the hot reload it doesn't matter too much as only devs see that.

Maybe min 5 icon width for top row would be better (assuming that fits STABLE version label)?

@kvakvs
Copy link
Collaborator Author

kvakvs commented Feb 21, 2020

I'll go with your original idea, didn't notice you had more complex logic there than just 4 button count, it was 4 buttons on top row which should be enough to fit version label.

@kvakvs
Copy link
Collaborator Author

kvakvs commented Feb 21, 2020

I've made a layout class which handles button placement and row breaks, it is not tested, i am at work and have no way to run the game. If you wish to play the happy debugging, you can take it now, otherwise i welcome you to try this branch later tonight.

@originalfoo
Copy link
Member

Tested in-game - the button rows/cols is working great. However the panel backgorund is glitching.

image

Panel background didn't update when adding 5th button:

image

At 8 buttons added it looks like panel background is being extended wrong way (or think there's extra row?)

image

9 buttons and panel height is correct but width still only 4 buttons wide:

image

10 buttons, panel bg too high again:

image

etc...

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.

I like it 👍 resizing works as described (missing translation keys are included in #723 ).

image

Copy link
Member

@originalfoo originalfoo left a comment

Choose a reason for hiding this comment

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

This is working really great now - LGTM 👍

@kvakvs kvakvs merged commit 68cdcc5 into CitiesSkylinesMods:master Feb 21, 2020
@originalfoo originalfoo modified the milestones: 11.2, 11.1.1 Feb 21, 2020
@originalfoo originalfoo added the Toolbar The main TMPE toolbar label Apr 11, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
code cleanup Refactor code, remove old code, improve maintainability technical Tasks that need to be performed in order to improve quality and maintainability Toolbar The main TMPE toolbar UI User interface updates
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants