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

Revamp Tiberian Dawn ingame UI #16359

Merged
merged 3 commits into from May 15, 2019

Conversation

@dragunoff
Copy link
Contributor

commented Mar 27, 2019

This PR implements a revamped ingame UI for Tiberian Dawn. The UI is based on work done by @xan2622 and @mabulsoud with some twists from myself. The observer UI remains the same, that could be tackled in a future PR.

Closes #7572
PR with design source files: OpenRA/ArtSrc#7

Changes

The second commit is quite large so here's a summary of the changes:

Images:

  • Revamp chrome.png
  • Add dialog.png (split from chrome.png)
  • Add bits.png (split from chrome.png)
  • Remove strategic.png (moved into bits.png)

YAMLs:

  • Revamp chrome.yaml and ingame.yaml
  • Minor changes to mod.yaml, editor.yaml and ingame-chat.yaml

cs:

  • Upadte image coordinates in CncLoadScreen.cs

GDI

gdi-chrome

  • GDI tooltips remain unchanged due to some limitations of the widgets code

Nod

nod-chrome

Links

PSD source files
Forum thread

@pchote

This comment has been minimized.

Copy link
Member

commented Mar 28, 2019

Great work! I'll follow up with an ingame / usability review later, but for now have a major technical comment.

I raised this issue in the earlier discussion, and I'm afraid I'm going to have to insist on it now that it has made its way to a PR: chrome.png is generated from chrome.svg and should never be edited by hand.

Please update chrome.svg to add anything that can be vector based, and then create a new 512x512 raster format file (ideally xcf or psd without any photoshop-specific features) for the pixel based sidebar backgrounds.

VRAM is precious, so please try not to duplicate UI elements unnecessarily. The sidebar artwork could be made much more efficiently by having a single shared frame, with smaller cutouts that are drawn on top ingame for the faction logo and the faction-accented buttons areas.

@dragunoff

This comment has been minimized.

Copy link
Contributor Author

commented Mar 28, 2019

Thanks @pchote ☺️ I will wait for any usability reviews before addressing the technical matters. I was not aware that chrome.svg existed. I'm not sure that I'd be able to pull it off entirely without Photoshop effects but I'll see what can be done.

The duplication of assets was intentional because I based that on TS chrome and I wanted to have distinct skins for each faction. That idea didn't quite work out but I decided to keep the layout.

Is it OK that I split off bits.png and dialog.png? I assume that I'll need to file a PR to the ArtSrc repo with source files for them?

@pchote

This comment has been minimized.

Copy link
Member

commented Mar 28, 2019

Hmm, I was sure that we had already talked about chrome.svg, but I must have been mistaken.

I'm not sure that I'd be able to pull it off entirely without Photoshop effects but I'll see what can be done.

The new elements are fine as raster art if that is the primary source. My main annoyance was with the glyphs and other pre-existing artwork.

Is it OK that I split off bits.png and dialog.png?

Texture samplers are even more precious than VRAM - rendering performance falls off a cliff if we use more than 8 textures in a frame, and this includes the texture atlases used for rendering everything in the world. Textures should only be split off if the benefits outweigh the potentially non-neglible performance degradation it may bring.

@Inq8

This comment has been minimized.

Copy link
Contributor

commented Mar 28, 2019

55062705-31ea0600-507f-11e9-864e-33e27633a73b

Great work, ultimate nitpicking but I don't think that highlighted icon is centered correctly, or the button itself is too big. (visually its sitting off to the left)

I think the defence stance is also not centered, although it seems less obvious.

@dragunoff

This comment has been minimized.

Copy link
Contributor Author

commented Mar 28, 2019

Great work, ultimate nitpicking but I don't think that highlighted icon is centered correctly, or the button itself is too big. (visually its sitting off to the left)

I think the defence stance is also not centered, although it seems less obvious.

You are correct! Good catch! The buttons have uneven width. I will fix that.

@pchote

This comment has been minimized.

Copy link
Member

commented Apr 21, 2019

Some UI comments, mainly focused on the command bar:

  • IMO the frame around the command bar is too chunky. It looks much nicer if the frame width is reduced by 2px:
    reduced
  • The stance buttons feel IMO too narrow, making them harder to select with the mouse. Can we adopt the same button sizes used in RA?
  • The chat dialog needs to move up a bit so that it doesn't overlap with the commandbar at narrower window sizes.
  • We still lose at Fitt's law by having the bar offset from the edge and having an unclickable frame - but this applies to all mods so IMO is out of scope of this PR.
  • C&Cs have always had the powerbar on the left, so having it swapped with the silo bar here feels wrong. Please swap them back.
  • The red button separator lines on the Nod sidebar feel too strong. Can we tone these down somehow?
  • IMO the faction logos feel a bit small in the radar bin. Is there a reason not to make them bigger?

If we can address these and my technical comments above then I think this should be good to go.

@dragunoff

This comment has been minimized.

Copy link
Contributor Author

commented Apr 21, 2019

Thanks for the review @pchote -- I have already started working on the technical side (reducing the number of sprites and the overall size of the sprites). I mostly agree with your comments and I will look into fixing the issues.

* IMO the frame around the command bar is too chunky. It looks much nicer if the frame width is reduced by 2px:

* The stance buttons feel IMO too narrow, making them harder to select with the mouse. Can we adopt the same button sizes used in RA?

* The chat dialog needs to move up a bit so that it doesn't overlap with the commandbar at narrower window sizes.

* We still lose at Fitt's law by having the bar offset from the edge and having an unclickable frame - but this applies to all mods so IMO is out of scope of this PR.

I agree with everything here.

* C&Cs have always had the powerbar on the left, so having it swapped with the silo bar here feels wrong. Please swap them back.

I was aiming for consistency with RA and the cash count seemed better with "$" at the front. However you're right that it feels wrong for CNC.

* The red button separator lines on the Nod sidebar feel too strong. Can we tone these down somehow?

Sure thing. I will try the darker red used on the UI panel borders.

* IMO the faction logos feel a bit small in the radar bin. Is there a reason not to make them bigger?

They look quite fine to me but I'll try making them a little larger.

@pchote

This comment has been minimized.

Copy link
Member

commented Apr 21, 2019

Don't feel the need to go overboard with micro-optimizing the sprite layout, keeping the existing chrome.png as-is and then adding a new (not too large) sprite file for the new artwork, trying not to be too wasteful with duplication and empty space should be good enough.

@dragunoff

This comment has been minimized.

Copy link
Contributor Author

commented Apr 21, 2019

I've already changed the old sprite a quite a lot. I want to make it more logical with separate areas for icons, dialog boxes, logos, etc. I will file a PR to the ArtSrc repo soon.

@pchote

This comment has been minimized.

Copy link
Member

commented May 4, 2019

Any new progress here @dragunoff? We're not far off a new playtest cycle, and it would be a shame to miss out on this.

@dragunoff

This comment has been minimized.

Copy link
Contributor Author

commented May 6, 2019

@pchote Yes, I'm working on it :-) I have combined everything into a single 1024x512 sprite and I'm working through the changes requested. Expect an update until the end of this week.

@dragunoff dragunoff force-pushed the dragunoff:feature/cnc-ui--dragunoff branch from 3020c9a to 4b41ff0 May 7, 2019

@dragunoff

This comment has been minimized.

Copy link
Contributor Author

commented May 7, 2019

I have addressed all the comments and updated this PR and filed OpenRA/ArtSrc#7 with the design source files. Here are some new screenshots:

cnc-ui-screenshots-gdi

cnc-ui-screenshots-nod

@pchote pchote added this to the Next Release milestone May 7, 2019

@dragunoff dragunoff force-pushed the dragunoff:feature/cnc-ui--dragunoff branch from 4b41ff0 to 4f89884 May 8, 2019

@dragunoff

This comment has been minimized.

Copy link
Contributor Author

commented May 8, 2019

Updated because the performance graph was overlaping the command bar.

@pchote

This comment has been minimized.

Copy link
Member

commented May 12, 2019

GDI tooltips remain unchanged due to some limitations of the widgets code

Fixed with 94793ca - please cherry pick this into the PR.

The ingame chat will need some deeper changes to the TextFieldWidget so is best left to a followup PR.

Can you please also rebase this on latest bleed? The GitHub UI isn't reporting a merge conflict, but this will break things if merged from the current base. Beware of #16530 though, which will affect this.

@pchote

This comment has been minimized.

Copy link
Member

commented May 12, 2019

I left a couple of requests on OpenRA/ArtSrc#7.

This is looking really great, and none of the requests should hopefully be too difficult, so I think this should be good to merge once they're done.

@dragunoff dragunoff force-pushed the dragunoff:feature/cnc-ui--dragunoff branch from 4f89884 to 2cfcf92 May 13, 2019

@dragunoff

This comment has been minimized.

Copy link
Contributor Author

commented May 13, 2019

Rebased, cherry-picked and updated the sprite. Here's a screenshot with faction tooltips in:

td-ui-revamp

@dragunoff dragunoff force-pushed the dragunoff:feature/cnc-ui--dragunoff branch from 2cfcf92 to 674897c May 14, 2019

@dragunoff

This comment has been minimized.

Copy link
Contributor Author

commented May 14, 2019

Updated to fix code style error.

@dragunoff dragunoff force-pushed the dragunoff:feature/cnc-ui--dragunoff branch from 674897c to 62a5eeb May 14, 2019

@dragunoff

This comment has been minimized.

Copy link
Contributor Author

commented May 14, 2019

Updated to match the latest changes to chrome.svg.

@pchote
pchote approved these changes May 14, 2019
Copy link
Member

left a comment

LGTM 👍

This suffers from #16530 but I would prefer we deal with that at the same time as the rest of the regressions instead of implementing something here that may need to be reverted or changed.

If I wanted to nit-pick I could raise some points about the chrome definition naming and unused faction variants, but this would open up a much larger can of worms that I don't think is worth dealing with.

@pchote pchote added the PR: Needs +2 label May 14, 2019

@dragunoff

This comment has been minimized.

Copy link
Contributor Author

commented May 14, 2019

If I wanted to nit-pick I could raise some points about the chrome definition naming and unused faction variants, but this would open up a much larger can of worms that I don't think is worth dealing with.

I deliberately included faction variants for all panels even though they are not used. I thought I'd better do it now that I am onto it. It may come in handy if we want to do anything with faction suffix later.

I noticed a few other unused definitions in chrome.yaml but I was not sure what the intentions were behind them so I left them untouched.

@pchote

This comment has been minimized.

Copy link
Member

commented May 14, 2019

a much larger can of worms 😄

@reaperrr reaperrr merged commit 72dbb87 into OpenRA:bleed May 15, 2019

2 checks passed

continuous-integration/appveyor/pr AppVeyor build succeeded
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
4 participants
You can’t perform that action at this time.