Opened 7 years ago

Closed 2 years ago

#1646 closed enhancement (fixed)

[PATCH] Show unmet requirements in technology icon tooltips

Reported by: zoot Owned by:
Priority: Nice to Have Milestone: Alpha 22
Component: UI & Simulation Keywords: patch
Cc: Patch:

Description

Show unmet requirements in technology icon tooltips.

See this thread for discussion.

Attachments (1)

techtiphack3.diff (5.4 KB) - added by zoot 7 years ago.

Download all attachments as: .zip

Change History (12)

Changed 7 years ago by zoot

Attachment: techtiphack3.diff added

comment:1 Changed 7 years ago by Ben Brian

Cc: patch review added
Component: Core engineUI & Simulation
Milestone: BacklogAlpha 12

comment:2 Changed 7 years ago by fcxSanya

Cc: patch review removed
Keywords: patch review added

comment:3 Changed 7 years ago by Ben Brian

Keywords: review removed

Some comments from looking at the patch:

  • At first I wasn't sold on the need for a custom tooltip formatting syntax, but looking at all the possibilities, it may be the best option.
  • We should probably go ahead and plan on handling multiple requirements per tech, which may need a recursive design of the tooltip string building function.
  • The required count appears red, as if it's an unmet requirement. It will remain red even when the count is satisfied, which is an inconsistency in the UI. The logic would only grow messier to solve that with conditional colors or visibility.
  • I think the logic in GetRequirementsVars() should be combined with getFormattedRequirementsTooltip()

I guess the question is: do we want to keep this as simple as possible and only handle the case of a single class requirement per tech (which could be easily special-cased), or do we want to make it super flexible and handle all possibilities? The former would certainly be a simpler, neater solution. I don't know if we plan any techs that require a more complicated solution.

comment:4 Changed 7 years ago by Kieran P

Milestone: Alpha 12Alpha 13
Priority: Should HaveNice to Have

Moved to the next release as further work is required.

comment:5 in reply to:  3 Changed 6 years ago by zoot

Sorry, I seem to have missed the last comments here.

Replying to historic_bruno:

  • We should probably go ahead and plan on handling multiple requirements per tech, which may need a recursive design of the tooltip string building function.

Would it not be better to do that when multiple requirements are implemented? Then we will know how the format will look.

  • The required count appears red, as if it's an unmet requirement. It will remain red even when the count is satisfied, which is an inconsistency in the UI. The logic would only grow messier to solve that with conditional colors or visibility.

This is not strictly true. When the count is satisfied, the check on unit_commands.js:673 will fail, and the string won't shown in the tooltip.

  • I think the logic in GetRequirementsVars() should be combined with getFormattedRequirementsTooltip()

GetRequirementsVars() is really just a glorified getter, modelled on the structure of CheckTechnologyRequirements(). The idea being that all the logic for parsing technology requirements should be contained in the component to which it is native.

I can copy it over if you insist, but then the logic has to be maintained in two different files. Alternatively, I could try to find a way to combine GetRequirementsVars() and CheckTechnologyRequirements() (or at least have one call the other)?

Version 0, edited 6 years ago by zoot (next)

comment:6 Changed 6 years ago by Kieran P

Milestone: Alpha 13Alpha 14

comment:7 Changed 6 years ago by Kieran P

Milestone: Alpha 14Alpha 15

comment:8 Changed 6 years ago by wraitii

Milestone: Alpha 15Alpha 16

comment:9 Changed 5 years ago by leper

Milestone: Alpha 16Backlog

comment:10 Changed 5 years ago by scythetwirler

It'd be nice to indicate which structures actually count as village/town phase structures on their individual structure tooltips for newer players.

comment:11 Changed 2 years ago by elexis

Milestone: BacklogAlpha 22
Resolution: fixed
Status: newclosed

In r19120 by fatherbushido:

Merge different logics of technology requirements and fix the related bugs. Create a global script which derive the technology requirements objects in a form usable for structure tree, gui, simulation and AI. Reviewed by mimo (for AI) and fatherbushido. Fixes #3993, #1646, #4263, #4217. Refs #4108.

In r19121 by fatherbushido:

Add a simulation test to the previous commit of s0600204's patch. Refs #4263.

Note: See TracTickets for help on using tickets.