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

Chinese theme; Fallback theme; New signs #1523

Merged
merged 21 commits into from
Apr 26, 2022

Conversation

kvakvs
Copy link
Collaborator

@kvakvs kvakvs commented Apr 6, 2022

Renames

  • RoadSignTheme<s> renamed to RoadSignThemeManager and extracted class RoadSignTheme into a file
  • RoadDefaults special theme (green speed limit signs) renamed to SpeedLimitDefaults
  • Priority sign filenames changed from Stop → PriorityStop, Yield → PriorityYield, RightOfWay → PriorityRightOfWay, and a new sign added to Fallback theme named PriorityNone (the gray circle).

Changes

  • Large diff due to committing SVG source file which is a XML and splitting a large C# file into 2 and renaming.
  • Vehicle restrictions and priority signs are now part of theme engine, removed them from RoadUI entirely. Translated stop/yield signs for Chinese are removed - use Chinese signs theme and it is not connected to Chinese language in the UI anymore.
  • Added Kmph_China road signs theme (source in SVG with fonts)
  • Added Kmph_China_Generic road signs theme based on Kmph_China and patches few signs by removing glyphs from them. Stop sign remains with a glyph.
  • Added Fallback theme which is used for vehicle restrictions, parking and priority signs as a default fallback
  • Fallback theme will have white texture instead of null if something fails to load.
  • Included vehicle restrictions for trains too

Need help: A chinese speaker to have a look, there are no road signs for Taxi and Emergency services, so i have translated them myself. (thanks to @lokpro)

bild
bild
bild
bild

EDIT (aubergine): Putting here for use in release notes re: crowdin translations (08/Apr/2022): AduitSSH, Aimarekin, John Lok Ho (lok91101), John Longo (john.j.longo), Dominik Kawula (dom.kawula), GiorgioHerbie, John Deehe (Deeheks), Lasm Gratel (LasmGratel), 문주원 (sky162178), PhoneixS, Chamëleon (chameleon-tbn), Skazov, Fizzy_LaFizz, shg166 (shg166_csl), Dmytro Lytovchenko (kvakvs), Patipan Doungjan (frame150911), Zeldslayer.

@originalfoo originalfoo added UI User interface updates Overlays Overlays, data vis, etc. labels Apr 7, 2022
@lokpro
Copy link

lokpro commented Apr 8, 2022

I am a Chinese speaker.
Is this theme for Mainland China (and may have themes for other parts of China) ?
If yes, you may leave the Chinese characters on the signs, they are good for Mainland China style.
If this a theme for a generalised Chinese style, as you wrote "not connected to Chinese language in the UI", the Chinese characters should be removed. I find them on "-Taxi.png" "-Emergency.png", "PriorityStop.png", "PriorityYield.png" and "China.svg".

@lokpro
Copy link

lokpro commented Apr 8, 2022

I did a search for signs of different Chinese styles.

Mainland China signs
https://www.google.com/search?q=%E4%B8%AD%E5%9C%8B+%E4%BA%A4%E9%80%9A+%E6%A8%99%E8%AA%8C&source=lnms&tbm=isch&sa=X&ved=2ahUKEwj_2bzkzIP3AhU_xosBHTl6BrEQ_AUoAXoECAEQAw&biw=1766&bih=1022&dpr=1

Taiwan signs
https://www.google.com/search?q=%E4%BA%A4%E9%80%9A%E8%99%9F%E8%AA%8C&source=lnms&tbm=isch&sa=X&ved=2ahUKEwjLycffy4P3AhUpxYsBHRECA7EQ_AUoAXoECAEQAw&biw=1766&bih=1022&dpr=1

  • "PriorityStop.png" (停) is common for both. keep it.
  • "PriorityYield.png" the characters are not common. maybe just use the default sign.

@kvakvs
Copy link
Collaborator Author

kvakvs commented Apr 8, 2022

I do not plan to do themes for dialects right now.
The selected themes were made by popularity on Steam from top to bottom.

Although it is probably possible to make a theme to "patch" other theme and replace a couple signs only.

P.S. Adding a second theme "China Generic" that will not have text on signs, only Stop 停.

@originalfoo originalfoo added the Localisation Localised text and features label Apr 8, 2022
@originalfoo
Copy link
Member

originalfoo commented Apr 8, 2022

@kvakvs CI borked due to something in OptionsManager: The modifier 'internal' is not valid for this item

CI log
C:\projects\cities-skylines-traffic-manager-president-edition-labs\TLM\TLM\Manager\Impl\OptionsManager.cs(336,9): error CS0106: The modifier 'internal' is not valid for this item [C:\projects\cities-skylines-traffic-manager-president-edition-labs\TLM\TLM\TLM.csproj]
C:\projects\cities-skylines-traffic-manager-president-edition-labs\TLM\TLM\Manager\Impl\OptionsManager.cs(350,9): error CS0106: The modifier 'internal' is not valid for this item [C:\projects\cities-skylines-traffic-manager-president-edition-labs\TLM\TLM\TLM.csproj]
C:\projects\cities-skylines-traffic-manager-president-edition-labs\TLM\TLM\Manager\Impl\OptionsManager.cs(367,9): error CS0106: The modifier 'internal' is not valid for this item [C:\projects\cities-skylines-traffic-manager-president-edition-labs\TLM\TLM\TLM.csproj]
C:\projects\cities-skylines-traffic-manager-president-edition-labs\TLM\TLM\Manager\Impl\OptionsManager.cs(384,9): error CS0106: The modifier 'internal' is not valid for this item [C:\projects\cities-skylines-traffic-manager-president-edition-labs\TLM\TLM\TLM.csproj]
C:\projects\cities-skylines-traffic-manager-president-edition-labs\TLM\TLM\Manager\Impl\OptionsManager.cs(396,2): error CS1513: } expected [C:\projects\cities-skylines-traffic-manager-president-edition-labs\TLM\TLM\TLM.csproj]

@originalfoo
Copy link
Member

originalfoo commented Apr 8, 2022

The original vehicle restriction icons are still in TLM\Resources... suggestion:

  1. Move new Chinese-style vehicle restriction fallback icons from TLM\Resources\SignThemes\Fallback in to TLM\Resources\SignThemes\Kmph_China_Generic
  2. Move original vehicle restriction icons (which users are familiar with) from TLM\Resources to TLM\Resources\SignThemes\Fallback
  3. Add a README.md in Fallback folder clarifying what the folder is for?
  4. Rename RoadDefaults folder to DefaultSpeedLimits or BaseSpeedLimits? And a README.md in there too?

@kvakvs
Copy link
Collaborator Author

kvakvs commented Apr 8, 2022

The original vehicle restriction icons are still in TLM\Resources... suggestion:
1. Move new Chinese-style vehicle restriction fallback icons from TLM\Resources\SignThemes\Fallback in to TLM\Resources\SignThemes\Kmph_China_Generic
2. Move original vehicle restriction icons (which users are familiar with) from TLM\Resources to TLM\Resources\SignThemes\Fallback
3. Add a README.md in Fallback folder clarifying what the folder is for?
4. Rename RoadDefaults folder to DefaultSpeedLimits or BaseSpeedLimits? And a README.md in there too?

Fallback does not and never contained Chinese signs, but i replaced old vehicle restrictions with similar style blue signs. Chinese signs have Chinese style vehicle silhouettes and some have chinese text on them, Fallback doesn't have that
bild

Original vehicle restriction icons are meant to go, we do not have high resolution source for them, too. But i am quite happy to give the allow icons green border instead of blue circle, making them very similar to previous signs. But green signs do not exist, blue signs are at least remotely realistic.

Adding README to Fallback and renaming RoadDefaults, though

@kvakvs
Copy link
Collaborator Author

kvakvs commented Apr 8, 2022

Dark green without inner white circle?
bild

Or bright green with white circle and invert icons to black like the original?
Why this inversion - i still care that some people have trouble distinguishing red and green, and only navigate by brightness of the lime signs. Having black and white icons for different states should help more.

@originalfoo
Copy link
Member

originalfoo commented Apr 9, 2022

Only issue I've found so far while testing (not sure if due to this PR or not) is that when I have speed limit tool open, pressing Esc causes game Pause Menu to appear rather than exiting Speed Limit tool.

EDIT: Other TM:PE tools do handle Esc properly, seems to only be Speed Limits tool that's affected.

EDIT: Only just noticed comments above - will respond to those in a bit.

@originalfoo
Copy link
Member

Original vehicle restriction icons are meant to go, we do not have high resolution source for them, too.

I'm fine with most of the changes. However for passenger train I'd prefer to keep original icon (PR #31) as it's much simpler and easier to comprehend at a glance than a passenger carriage. The hi-res source can be obtained from Noun Project: https://thenounproject.com/icon/passenger-188591/

I think the blue background for "vehicle allowed" was better than the green as it has slightly better contrast with the white vehicle icon in foreground. Also, many countries use blue background with white vehicle foreground to indicate when lane is dedicated to certain vehicle type, and while not a direct facsimile of TM:PE vehicle restrictions I think it would still help to keep that style to aid user comprehension of the feature.

Example from UK

image

@kvakvs
Copy link
Collaborator Author

kvakvs commented Apr 9, 2022

bild
Got these, 2 figures i really like better than 1

@TianQiBuTian
Copy link
Contributor

I found some more standard signs. Can be used as a reference.
China (Mainland): (Specified in GB 5678-2009)
https://en.wikipedia.org/wiki/Road_signs_in_China
China (Hong Kong):
https://en.wikipedia.org/wiki/Road_signs_in_Hong_Kong
China (Macao): (only pictures)
https://commons.wikimedia.org/wiki/User:Fry1989/Gallery/Road_Signs/Macau
China (Taiwan): (only pictures)
https://commons.wikimedia.org/wiki/User:Fry1989/Gallery/Road_Signs/China#Republic_of_China_(Taiwan)

In addition, the Simplified Chinese of taxi is "出租车" instead of "出租車". "出租車" is Traditional Chinese.

@originalfoo
Copy link
Member

I really like the hopper cargo traincar, and i have its origin recorded too. But i can go with replacing cargo train with cargo on hook, so that it could be used in both airport and train track situations

Ok, what about:

The cargo hook icon might be better suited to cargo harbours should we ever add support for ships (would be cool to have something BFS search algorithm for cargo harbours).

@krzychu124
Copy link
Member

The cargo hook icon might be better suited to cargo harbours should we ever add support for ships (would be cool to have something BFS search algorithm for cargo harbours).

You can't use BFS or any segment iterating algorithm if there are no segments 😂 I don't know the details but I'm pretty sure it works more or less like for airplanes when they leave airplane path while trying to land at airport - they move towards the target that looks more or less like Proportional navigation/homing missile algorithm

@kvakvs
Copy link
Collaborator Author

kvakvs commented Apr 17, 2022

Saved hooked cargo on the page side, for future use
Reverting cargo train to the cargo hopper icon
Forklift i can add the icon to the SVG source, but not export it until it is time.

@originalfoo
Copy link
Member

re: forklift - I found some scissor lift icons that would perhaps be better suited to airports: #1458 (comment)

@kvakvs
Copy link
Collaborator Author

kvakvs commented Apr 17, 2022

I found some more standard signs. Can be used as a reference. China (Mainland): (Specified in GB 5678-2009)

I used this Wikipedia page to make the first version of the theme. But then some signs in TM:PE do not have standard signs in all countries, so i had to improvise.

https://en.wikipedia.org/wiki/Road_signs_in_China China (Hong Kong): https://en.wikipedia.org/wiki/Road_signs_in_Hong_Kong
China (Macao): (only pictures) https://commons.wikimedia.org/wiki/User:Fry1989/Gallery/Road_Signs/Macau
China (Taiwan): (only pictures) https://commons.wikimedia.org/wiki/User:Fry1989/Gallery/Road_Signs/China#Republic_of_China_(Taiwan)

Do we need special themes for those territories? I was choosing themes based on potential count of players.

In addition, the Simplified Chinese of taxi is "出租车" instead of "出租車". "出租車" is Traditional Chinese.

This moment was unclear, i guess we're doing simplified rather than traditional.

@kvakvs
Copy link
Collaborator Author

kvakvs commented Apr 17, 2022

re: forklift - I found some scissor lift icons that would perhaps be better suited to airports: #1458 (comment)

Got this for now. It is not exported or not used anywhere, just a proposed icon which vaguely matches our other vehicles by their wheel shape, so that they look nicely together.
bild

@chameleon-tbn chameleon-tbn self-requested a review April 20, 2022 08:18
Copy link
Contributor

@chameleon-tbn chameleon-tbn 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 the new style a lot. Very good work!
code lgtm

But as a comment on this: I had discussed with @krzychu124 also Planes, Cyclists and Pedestrian signs for future development (not only in TMPE, but as it will be something to manage in TMPE it would be nice to have same style for them too)... can you create them in same style? @kvakvs

@kvakvs
Copy link
Collaborator Author

kvakvs commented Apr 20, 2022

But as a cooment on this: I had discussed with @krzychu also Planes, Cyclists and Pedestrian signs for future development (not only in TMPE, but as it will be something to manage in TMPE it would be nice to have same style for them too)... can you create them in same style? @kvakvs

The pictures are stored as SVG (that's a text file in Git), and you can edit using Inkscape (free software).
Also I can add more signs no problem.

@krzychu124
Copy link
Member

krzychu124 commented Apr 23, 2022

@kvakvs I assume that white squares instead of some default speed limits for China_generic is a bug, right?

Edit:

NOTE: Speed limit road signs do not query Fallback theme, it is assumed that
any current theme would provide all necessary numbered road signs.

From the log

Debug 602.7954460: Resource SignThemes.Kmph_China_Generic.0.png not found (not an error)
//...
Debug 603.1673537: Resource SignThemes.Fallback.0.png not found (not an error)
//...
Info 603.2467477: Road Sign theme changed to Kmph_China_Generic

@kvakvs
Copy link
Collaborator Author

kvakvs commented Apr 23, 2022

@kvakvs I assume that white squares instead of some default speed limits for China_generic is a bug, right?

Edit:

NOTE: Speed limit road signs do not query Fallback theme, it is assumed that
any current theme would provide all necessary numbered road signs.

From the log

Debug 602.7954460: Resource SignThemes.Kmph_China_Generic.0.png not found (not an error)
//...
Debug 603.1673537: Resource SignThemes.Fallback.0.png not found (not an error)
//...
Info 603.2467477: Road Sign theme changed to Kmph_China_Generic

China Generic theme is BASED ON China, and then China is BASED ON Fallback theme, so white squares are a bug, but "loading error" is not a bug, because it would look 1 and 2 levels down in the base themes. Looks like <theme>/0.png is missing and that i can fix quickly.

@krzychu124
Copy link
Member

Hmm, China (Mainland) works normally, so no idea why generic is not. Other than that no issues, looks good. Will do another test once fixed.

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.

Looks good, works in game without noticeable issues.

@@ -406,7 +409,8 @@ public VehicleRestrictionsTool(TrafficManagerTool mainTool)
selectedSegmentInfo,
selectedLaneIndex,
selectedLaneInfo,
VehicleRestrictionsMode.Configured); ;
VehicleRestrictionsMode.Configured);
;
Copy link
Member

Choose a reason for hiding this comment

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

Not needed (;), I think 😅

/// Singleton which manages road signs themes dynamically loaded and freed as the user .
/// The textures are loaded when OnLevelLoaded event is fired from the <see cref="Lifecycle.TMPELifecycle"/>.
/// </summary>
public class RoadSignThemeManager : AbstractCustomManager {
Copy link
Member

Choose a reason for hiding this comment

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

I would run some code cleanup e.g: Rider -> Code -> Rearrange code to organize things a bit because now everything is mixed. Some private fields don't follow naming convention (wrong case), some other could be readonly etc.
Sometimes I feel like line width is too small (in general, not here), especially in places with nested if's or debug logs where formatter break lines like mad creating walls of text for half page so 90% of time you scroll to search for the actual code 😂

@TianQiBuTian
Copy link
Contributor

Oh, I found the newly released regulation document (GB 5768.2-2022).
So do you need to change the icon according to the regulation document?

@kvakvs
Copy link
Collaborator Author

kvakvs commented Apr 25, 2022

GB 5768.2-2022

Wikipedia signs are 2009 (GB 5678-2009.)
If you have a link with all road signs, would be great if they had SVG sources like Wikipedia page. But no SVG is ok too.
Or we can update it later, when these new signs are installed in many places?

@kvakvs kvakvs merged commit 37307df into CitiesSkylinesMods:master Apr 26, 2022
@originalfoo originalfoo added this to the 11.6.5.2 milestone Apr 27, 2022
@Citrinae-Lime
Copy link

Wikipedia signs are 2009 (GB 5678-2009.)

I have uploaded a pdf of the GB 5678.2-2022, it may be of some help.

@kvakvs kvakvs deleted the theme/chinese-signs branch June 4, 2022 16:26
@kvakvs
Copy link
Collaborator Author

kvakvs commented Jun 4, 2022

I will make a new task for updating signs, or maybe have China mainland and China Mainland 2022 themes, why not.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Localisation Localised text and features Overlays Overlays, data vis, etc. UI User interface updates
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

7 participants