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

Upgrade observer/replay interface features. #5121 #13023

Closed
wants to merge 1 commit into from
Closed

Upgrade observer/replay interface features. #5121 #13023

wants to merge 1 commit into from

Conversation

rob-v
Copy link
Contributor

@rob-v rob-v commented Mar 25, 2017

Statistics window restyled, each tab has hotkey configurable in Settings dialog. Scrollbar added when there is more than 8 players. F1 - show/hide Statistics (previous shown tab is displayed).
In TD there is added number of currently produced items in Production tab. After these changes, more UI improvements can be done simpler.

obrazok
obrazok

obrazok

}
}

int SortGroupByInfoType(ProductionItem queue)
Copy link
Contributor

Choose a reason for hiding this comment

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

WHAT?!

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Something more useful?

Copy link
Contributor

Choose a reason for hiding this comment

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

Arbitrary hardcode. I have no idea where this was used or why this was even added but this flatout goes against the direction OpenRA's supposed to take lately.

Copy link
Contributor

Choose a reason for hiding this comment

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

IOW: this probably masks a bug instead of resolving an issue.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

That SortGroupByInfoType can be removed, it is optional. I don't like it also. There was comment to display items in this order - Building, .. Infrantry, Vehicle, Aircraft. Do you have an idea how to sort it in general?

Copy link
Contributor

Choose a reason for hiding this comment

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

I don't see what was the issue with the legacy sort-queues-in-their-order-of-enabling. If that has issues with mods, that's the mod's fault.

Copy link
Contributor Author

@rob-v rob-v Mar 25, 2017

Choose a reason for hiding this comment

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

For D2K it was in different order than other mods. I could keep it like it was, however it will be now more difficult with that more general code for multique support. As there are many buildings in various orders and I implemented it to display in this order:
building, defence, infantry, vehicle. With sort-queues-in-their-order-of-enabling it won't be sorted.

Copy link
Contributor

@GraionDilach GraionDilach left a comment

Choose a reason for hiding this comment

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

This PR smells of design issues, ignores the issue of third-party mods ending up with overlapped support powers and production issues because the end result "looks nicer" in the shipped mods.

@rob-v
Copy link
Contributor Author

rob-v commented Mar 25, 2017

Only SortGroupByInfoType smells and can be removed and I expected some useful comments in this PR to resolve it. Otherwise it is more general than it was. Any mod with single or multique will work - like currently it works for RA and TD. So it is better than it was and I hope it will be better basis for next improvement.

@rob-v
Copy link
Contributor Author

rob-v commented Mar 25, 2017

SortGroupByInfoType didn't cause bug for new mods, it would display unknown types as not sorted, however it isn't general so this enforced ordering - Building, Defense, Infantry - was removed and replaced by sorting by ProductionQueueInfo.Type.
If you know about better general way how to sort it, let me know.

@SoScared
Copy link
Member

SoScared commented Mar 26, 2017

On production stacking, unit production put 'on hold' takes priority in the slots over active production in circumstances which 'on hold' is closer to finish compared to active ones. Thus 'on hold' production ought to be prioritized lower than active production.

If possible (polish beyond the issue) the 'unit complete' animation sequence should be removed/skipped from the production info for visual sake, more so with stacked production as it slightly "flashes" when going from one producing icon to the next.

For whatever reason, when re-assigning hotkeys for the info tabs I have to quit and restart the replay in order for the new assignment to take effect.

@SoScared
Copy link
Member

SoScared commented Mar 26, 2017

I ran it through some Debug "stress tests" and noticed a few discrepancies:
ra
td

A 1-pixel glitch on the top right corner (all mods) due to I'm guessing the hidden scroll bar area.

The exposed overlap on TD demonstrated above (includes 10 production w/o stacking) is (if I understand the issue correctly, expressed in #5121 ) something to look out for in regards to future mods for OpenRA. For TD itself it won't be visually intruding due to the nature of TD's eco in addition to the new stacking feature. Here's an "high end" production example of 11 items produced on 8 slots (9 slots max for TD) for the current PR:
td2

D2K looks to need one additional production slot (7 in total) to cover all tabs - currently D2K team games is a spamfest and there's a reasonable chance players get to experience full production capability from time to time.

TS production tab looks gorgeous, only keeping note on should TS sometime in the future adopt a multi-queue system as TD the production bar would have to be adjusted accordingly.

I believe the rows on RA/TD would look a bit more attractive with slightly more space between the slots, e.g. 5 pixels space in-between (instead of 3) horizontally and 7-8 pixel space in-between vertically (instead of 5). This would make the field slightly bigger but I think design wise it's worth it.

@SoScared
Copy link
Member

SoScared commented Mar 26, 2017

Otherwise I gotta say I'm super stoked to finally be able to experience this sort of visual stats display! 👍

@SoScared
Copy link
Member

SoScared commented Mar 26, 2017

And just to heap on more work, the TD replay clock would likely need adjustment. Suggestion: here's a MS Paint mock-up of the clock 90% of it's original size with the flashing speed indicator positioned as an appendix. With an added production slot to D2K as suggested above, this ought to be done there as well.
td3

@rob-v
Copy link
Contributor Author

rob-v commented Mar 26, 2017

Thanks a lot for your quick/great test and comments, I was shocked, didn't expect it.
Fixed 1-pixel glitch on the top right corner. Increased spacing between icons and rows. Copied and adjusted .yaml for D2K to have different layout. Added - used ScrollItem/Tab width from .yaml -> removed dependency on last column name - simpler code and customization.
D2K - adapted (thanks to new .yaml) to more (7?) slots. All mods share same code for Production - so in case of TS change, it should work like for TD (we only need to change e.g. width in .yaml).
Hotkeys - yes, they are assigned at start, otherwise they would need to be retrieved on each key press, so unnecessary usually. Can we keep it?

@SoScared
Copy link
Member

SoScared commented Mar 26, 2017

Let's just say personally I'm highly motivated on the matter plus the predecessor issues dates as far back as early 2014^^

The new spacing between the icons looks great! Now off course that has displaced the position vs the support power icons :)

I'll be more specific on the max. likely needed amount of production/support power slots per mod.

RA: 6 production + 5 support powers (6 if Ukraine having all support powers + a capped Chronosphere). I dont believe RA will practically benefit from having 6 slots instead of 5 due to it being infrequent with exception to modded maps. Imo design wise 5 would look the best. Current look:
ra

TD: 9 production + 2 support powers (3 if a faction caps the other factions' t3 tech) following the logic behind RA's slot count. Current look:
td

D2K: 7 production + 2 support powers (4 if a faction caps the 2 other factions' t3 tech) following the logic behind RA's slot count. Current look:
d2k

TS: 5 production + 3 support powers. Current look 👍 :
ts

@SoScared
Copy link
Member

SoScared commented Mar 26, 2017

Also I think the right end side might benefit from a standard space distance due to the show/hide scroll bar (unless there's a custom function to resize the space distance to whether or not there will be a scroll bar following the player count). A standard format could look like this (NB! MS Paint mock-up):
end-bar
With the standard distance between the last slot and the right edge being 32 pixels wide. Without the scroll bar I believe 8 pixels would have been sufficient.

*edit: On a side note, a ca. 8 pixel distance between the 'production' and 'support power' slot rows would look consistent as well.

@SoScared
Copy link
Member

SoScared commented Mar 26, 2017

It is currently sorted by remaining time, it means READY, then in progress or ON HOLD. You mean you want to display active item before ON HOLD item, even when ON HOLD has less time remaining and when unpaused could finish earlier? So it will be sorted by state (READY, ACTIVE, ON HOLD) and then by time?

Yeah that sounds about right. Otherwise the production bar would skip a lot if players hold the production on some facilities while keeping production going for others. E.g. multiple rifle infantry production on TD expose this quickly.

@SoScared
Copy link
Member

SoScared commented Mar 26, 2017

Finally, if possible I think it would be wise for the mod specific production bars to mask out icon slots that surpass the slot limits so as to not garble the display in the fringe cases the production/support powers actually surpass the custom slot counts.

@SoScared
Copy link
Member

SoScared commented Mar 26, 2017

So it will be sorted by state (READY, ACTIVE, ON HOLD) and then by time

Actually, if I read this correctly, prioritizing 'READY' behind ACTIVE would eliminate the annoying short 'ready' flash when the progress bar jumps from one icon to the next in the stacked slots. So maybe (ACTIVE, READY, ON HOLD)?

I think I hinted at removal of 'READY' somewhere above but it seems like a necessity to indicate produced structures that are waiting to be placed.

@rob-v
Copy link
Contributor Author

rob-v commented Mar 26, 2017

@SoScared I see now, you are not so scared, you scare people. You forced me to calculate all those x+y, pixels here and there in Excel, but it is much better now for me also to adjust all if needed.
I increased spacing and also adjusted width, but I worked with different x prod. + y support numbers so it was worse at the end.
If there are more prod. icons than available space, they are now skipped. (Lately) In case the mod could provide number of possible/recommended queues, the width could be adjusted automatically, now it has to be done in .yaml manually. I was also thinking about making half size icons to be displayed in the same row when there are more icons needed. Other rows and support powers would stay in current size.
The Paused/Speed indicator was moved to the right of timer.
It is now sorted by ready, active, on hold as you requested, what is really better than always by remaining time.

@SoScared
Copy link
Member

SoScared commented Mar 26, 2017

@rob-v I like to believe I used to scare people. I've been out of consistent practice for over a year so now everyone just laugh at me 😢

@rob-v
Copy link
Contributor Author

rob-v commented Mar 26, 2017

With little training here, it will be fine again.
Fantastic idea - it seems with your idea to sort by active, ready, on hold, the flashing disappeared.

Copy link
Member

@pchote pchote left a comment

Choose a reason for hiding this comment

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

The current approach breaks down in multi-queue mods if a player is producing too many unique things at once:

screen shot 2017-03-26 at 17 18 05

The simplest and IMO clearest way to solve this problem is to give each queue type its own slot (i.e. the TD observer interface will show at most 1 building, one defense, one infantry, etc), and then alternate the icon shown in each slot if more than one thing is being produced (i.e. if a player has two barracks, building 1 minigunner and 1 engineer, then the interface will show the minigunner for 1 second, then the engineer for 1 second, then repeat). The UI could show a little N / M text indicator below the icon to indicate which queue # the displayed icon is being produced from.

@SoScared
Copy link
Member

Third run-through. I also deliberately increased production on TD to 12 unique units @pchote , the latest commit apparently has that issue sorted out by masking additional production slots over 9:
td_2
Support powers would benefit the same feature on all the mods (I can barely see a third support power icon in the above image after granting a faction t3 tech of the opposite faction).

As for the rest:

RA:
ra
ra_2

D2K:
d2k

TS:
ts

Now as for code implications with future mods I obviously can't speak to that but design wise this is looking pretty AAA to me.

Additional issues regarding UI design might include the game clock position and text style for each mod.

@pchote
Copy link
Member

pchote commented Mar 26, 2017

I really don't like the idea of simply adding more slots to TD, because it just defers the problem (and mods will inevitably find a way to break it) and means that we have a lot of wasted UI space in the common case.

@rob-v rob-v closed this Mar 26, 2017
@SoScared
Copy link
Member

SoScared commented Mar 26, 2017

Oh no. No no no no I did not wait 3 years for such a A grade stellar opportunity to be dropped with the blink of an eye. Nope! Absolutely negative, not happening.

@rob-v I understand the frustration of having done the significant amount of work in such a tiny time frame and then to have to go through additional levels of reviews. I haven't done any real code-work whatsoever but I think I can relate still with some of my more superfluous PR's in the past. Just stick through the storm until there's a definite conclusion.

@abcdefg30 abcdefg30 added this to the Future milestone Mar 26, 2017
@SoScared
Copy link
Member

SoScared commented Mar 26, 2017

All right I'm gonna stretch my neurons here to the moon and back so let me see if I can get the argument going forward.

disclaimer: I know jack about coding.

@pchote So the problem is additional mods coming in with their own production capacity potentially not fitting the related mod's features. Case in point Crystallized Doom's multi queue system providing approximately the same amount of production as the official TD mod, however being tied to the TS mod by default it would inherit the default setup provided for TS thus limiting the usefulness of CD's production info tab's display. Same issue would go for every other custom mod out there related to any of the official mods with a different production setup.

The alternative is to implement a one-size-fits-all with functionality that would be able to deliver the information uniformly by allowing different units to be displayed with the same slot, solving the need for excessive amount of slots by fitting objects into pre-determined slot types designated as 'infantry', 'vehicles', 'aircraft' etc. Two or more different vehicles produced simultaneously would then be represented by the one slot alternating back and forth between the two until the slot is back onto 1 unit type. The advantage would be for mod creators to have a functional production info bar display available from the get go.

An issue with a uniformed system is the accuracy of portraying real-time information, especially with multi-queues where 3-4 production facilities are represented by a single slot. For units/structures with long build times the result could be satisfactory but the quicker the unit is being produced the harder it would be to follow the information on the info bar. Multiple barracks producing a variety of infantry or production of light vehicles (normally fast built) could be problematic as the slot would likely be noisy at best.

The current proposed setup would demand of mod creators to alter the setup to fit their mods production should their mod deviate on that point vs the official mod it is built upon (or a common neutral version). This would require the modder to learn how to modify the production UI themselves, possibly from a pointer found in some documentation somewhere. This would put a certain amount of stress upon the mod creator depending on the procedure to which the production UI is custom tailored towards the mod and to the extent it would have been modified.

The advantage with having the production UI demand the modder to customize it would be for the UI to present the information tailored to the mods production system and deliver the information plain to the user (assuming that the modder will have to adjust the UI in the first place). Reliable and viewer friendly production UI custom tailored to the specific mod would better aid the mods promotional value by familiarizing the user more easily to the units, production style and game progression for the games.

Uniform production info UI
➕ Easy on modders
➖ Limits information

Custom production info UI
➕ Accurate information
➖ Modders needs to engage the UI

This whole comment rests on it actually making sense to which I more or less took a shot in the dark.

@SoScared
Copy link
Member

SoScared commented Mar 26, 2017

As I am personally a RA fanatic it wouldn't affect me too much one way or the other (RA using a single queue system) but I'm crossing my fingers be for the production UI to remain customized. I can only hope that there are easy solutions out there regarding UI coding for modders that would make it easier to support that position. Games with relatively obscure, unfamiliar units such as with Crystallized Doom would benefit more from having a production UI that at all times presents its production in a plain and straight-forward manner.

@SoScared
Copy link
Member

SoScared commented Mar 26, 2017

Regardless, with the current PR its initial solution (as an alternative at least) seem to just have these few "easy" polish points left:

  • Mask support-power icons that goes beyond the designated number of slots (as with production icons).
  • Sort stacked slot priority as (ACTIVE, READY, ON HOLD) to smoothen the stacked production visually.
  • Prioritize aircraft last in the production slot rows across all mods.
  • Evaluate text style, clock position in relation to the UI area.
  • Add documentation as to how to adjust the production stats UI slots to accommodate custom mods.

@MunWolf
Copy link
Contributor

MunWolf commented Mar 26, 2017

You can also just add a scrollable thing at the ends if the icons go past the given space can't you? I feel like that would be the best way to tackle the problem with mods being able to break it with a lot of queues.

@SoScared
Copy link
Member

@wolfgaming horizontal scrollbar was mentioned briefly with #5121 (comment). I think someone would need to come up with one.

@GraionDilach
Copy link
Contributor

I suggested the second row for support powers back then because that was feasible, requires no crazy stuff like stacking and can be applied to all mods and even compatible with @pchote's concerns as well.

@SoScared
Copy link
Member

SoScared commented Mar 27, 2017

It's not crazy at all, in fact it's being used by one of the most successful modern RTS games the past 8 years:

dzj84

Related: #5121 (comment)

I wouldn't presume to know the implications (although I try) of such a feature to OpenRA but as an observer tool there can be little doubt of its superiority when it comes to reviewing games and spectating or casting games.

...can be applied to all mods and even compatible with @pchote's concerns as well.

Aren't these two concerns one and the same? If you're referring to the overlapping issue that has already been resolved.

@rob-v rob-v deleted the SpectatorOverlay branch March 29, 2017 18:35
@abcdefg30 abcdefg30 removed this from the Future milestone Feb 7, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

6 participants