Skip to content

Create gui/spectate#1365

Merged
myk002 merged 23 commits into
DFHack:masterfrom
TimurKelman:gui/tooltips/1
Feb 22, 2025
Merged

Create gui/spectate#1365
myk002 merged 23 commits into
DFHack:masterfrom
TimurKelman:gui/tooltips/1

Conversation

@TimurKelman

@TimurKelman TimurKelman commented Jan 4, 2025

Copy link
Copy Markdown
Contributor

Configuration UI for the spectate plugin

image

@myk002

myk002 commented Jan 6, 2025

Copy link
Copy Markdown
Member

I am intrigued and delighted : )

@myk002

myk002 commented Jan 6, 2025

Copy link
Copy Markdown
Member

Let me finish reviewing DFHack/dfhack#4959 so getUnitsInBox no longer returns inactive units

image

Comment thread gui/tooltips.lua Outdated
@@ -0,0 +1,281 @@
-- Show tooltips on units and/or mouse

local RELOAD = false -- set to true when actively working on this script

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

FYI, devel/clear-script-env has the same effect:

multicmd devel/clear-script-env gui/tooltips; gui/tooltips

Comment thread gui/tooltips.lua Outdated
Comment thread docs/gui/tooltips.rst Outdated
:tags: fort inspection

This script shows "tooltips" following units and/or mouse with job names.

@myk002 myk002 Jan 6, 2025

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

needs disclaimer that tooltips will show over any vanilla UI elements:
image

The dig ascii overlays suffer from the same problem. I don't know of any good solution here. I'm not saying that any behavior needs to change -- just needs to be documented.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

added an **IMPORTANT NOTE** at the beginning of the description text

Comment thread docs/gui/tooltips.rst Outdated
Comment thread docs/gui/tooltips.rst Outdated
:summary: Show tooltips with useful info.
:tags: fort inspection

This script shows "tooltips" following units and/or mouse with job names.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

needs more description of the two options and their effects.

@TimurKelman TimurKelman Jan 7, 2025

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

added a bit of clarification...
It's kind of hard to describe though

Comment thread gui/tooltips.lua Outdated
Comment thread gui/tooltips.lua Outdated
Comment thread gui/tooltips.lua Outdated
Comment thread gui/tooltips.lua Outdated
Comment thread gui/tooltips.lua Outdated
TimurKelman and others added 2 commits January 6, 2025 11:16
Co-authored-by: Myk <myk.taylor@gmail.com>
Co-authored-by: Myk <myk.taylor@gmail.com>
@myk002

myk002 commented Jan 6, 2025

Copy link
Copy Markdown
Member

Btw this PR has started some discussion on the Discord server where your input would be valuable: https://discord.com/channels/793331351645323264/807444515194798090/1325754489659195502

@myk002 myk002 left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

My feedback:

  • this would be better as a fullscreen overlay that could be toggled via a keybinding bound to dwarfmode/Default and dungeonmode/Default (file). It could also be toggleable from gui/tooltips (which becomes just the configuration interface instead of the rendering mechanism). I don't want to promote leaving a windowed DFHack tool on the screen indefinitely because of DFHack/dfhack#4183
  • the enabled state should be managed by the overlay framework. this means that whether the tooltips are displayed is a per-player global setting. The tooltip behavior/contents configuration would likewise be global, stored in dfhack-config/tooltips.json

There were some reservations about the name gui/tooltips -- I'll get more feedback from people about that on Discord.

not required for this PR but maybe consider as a future enhancement:

  • the configuration for which elements to display could be a matrix of checkboxes so players could choose any combination of information for the tooltip and the mouseover.

@TimurKelman

Copy link
Copy Markdown
Contributor Author
  • the configuration for which elements to display could be a matrix of checkboxes so players could choose any combination of information for the tooltip and the mouseover.

is there a CheckBox control, or what should I use for this?

@myk002

myk002 commented Jan 15, 2025

Copy link
Copy Markdown
Member

There isn't a super convenient widget for that yet, largely because every place I've needed toggle checkboxes so far, they've been in a list (which doesn't take widgets). See gui/control-panel or maybe the prototype code in gui/manipulator for examples of the list-based approach. You could likely reuse the graphic assets and just slap them on a Label for a non-List option.

@TimurKelman

Copy link
Copy Markdown
Contributor Author

Config UI with current defaults

image

image

Comment thread gui/tooltips.lua Outdated
@myk002

myk002 commented Jan 20, 2025

Copy link
Copy Markdown
Member

should probably ensure the mouseover panel renders over the other floating text:
image

@myk002 myk002 left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

as discussed on Discord, could you rename gui/tooltips to gui/spectate and move the overlay code to plugins/lua/spectate.lua? spectate itself will need a bit of an overhaul. I can do that or you can if you'd like to.

Comment thread gui/tooltips.lua Outdated
Comment thread gui/tooltips.lua Outdated

-- pens are the same as gui/control-panel.lua
local textures = require('gui.textures')
local function get_icon_pens()

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

this is ugly duplication, but we don't have a good widget API for it yet, so I don't think there's a better option. We can change this later once we have a better way to address this fairly common use case. I see orders.lua does some similar duplication for the workshop labor restrictions overlay.

@TimurKelman

TimurKelman commented Jan 20, 2025

Copy link
Copy Markdown
Contributor Author

should probably ensure the mouseover panel renders over the other floating text

yeah, that might be a good idea :) But how?

@myk002

myk002 commented Jan 20, 2025

Copy link
Copy Markdown
Member

order of rendering in render()

@TimurKelman

Copy link
Copy Markdown
Contributor Author

as discussed on Discord, could you rename gui/tooltips to gui/spectate and move the overlay code to plugins/lua/spectate.lua? spectate itself will need a bit of an overhaul. I can do that or you can if you'd like to.

I'll try to do that, just can't promise when.

@myk002

myk002 commented Jan 20, 2025

Copy link
Copy Markdown
Member

I can overhaul spectate in the 51.01-r2 release timeframe. that would give us about a month to get this all finalized and merged

@TimurKelman

Copy link
Copy Markdown
Contributor Author

config UI is now a list:

image

@TimurKelman

Copy link
Copy Markdown
Contributor Author

btw, in case anyone is wondering: I haven't forgotten about merging this with spectate, I'm just trying to finish the new stuff first.

@myk002

myk002 commented Feb 2, 2025

Copy link
Copy Markdown
Member

no worries! I haven't done my part on spectate yet anyway, so it's not quite ready for you

edit: spectate is ready for you now. I added stubs and empty files in preparation

@myk002 myk002 changed the title Create gui/tooltips.lua: show info (f.e. job name) at units and/or mouse cursor Create gui/spectate Feb 10, 2025
@TimurKelman

Copy link
Copy Markdown
Contributor Author

some things there might deserve to be moved to some more central location:

  • ToggleLabel is like ToggleHotkeyLabel, but uses icon instead of "On"/"Off" text. Maybe make it an option?
  • pairsByKeys is a trivial table iterator in key order (with an optional sorting function). I didn't find one; this one I had to write twice (here and in the plugins/spectate.lua)
  • rpad is just return string.format("%-"..i.."s", s)... which may be too trivial
  • append (which should maybe called concat) concats two sequences together (which is also trivial enough)

@myk002

myk002 commented Feb 22, 2025

Copy link
Copy Markdown
Member

I'll see what I can upstream, then I'll clean things up

first upstreamed change: DFHack/dfhack#5296

@myk002 myk002 merged commit bbf75a7 into DFHack:master Feb 22, 2025
@TimurKelman

Copy link
Copy Markdown
Contributor Author

image

that's a lot of red :)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

No open projects
Status: Done

Development

Successfully merging this pull request may close these issues.

2 participants