CC: Tweaked

CC: Tweaked

65M Downloads

feat(window): Add window:getParent() and term:getNative()

Closed this issue · 4 comments

commented

Haii... I have a silly feature request - adding a new method getParent() to the window objects returned by window.create.

The current idea is to extend Window object with window:getParent() -> term.Redirect. Though, this may not be the best solution, so :getNative() may be required too if we add :getParent()

Adding window:getParent()

What issue are we trying to solve?

This method will allow the end user to figure out where the window is running - monitor or terminal, and the absolute position without a manual need of tracking pos or/and parent type. This acts as a core building block for solutions the end user may want to do.

People had issues such as getting absolute position of a window, e.g.:
#1561

I personally experienced trouble tracking click/touch positions in windows, especially with windows inside windows, I would require end user to track the window position themselves. I ended up monkey-patching window:getParent() to window.create(...) & window:reposition(...), but I think CC community needs this feature.

I consider this to be the most fundamental building block that allow end users to develop functions like getAbsolutePosition(window) -> x, y by iterating (nested) parents of windows.

Is this the best way to solve this issue?

Let's see the actual code:

function window.getParent()
    return parent
end

It does not change anything, only exposes the parent object. Minimal API surface, minimal complexity, no opinionated functions, high flexibility for the end user.

Issues with only "window:getParent()" solution

1. Multishell:

Since multishell uses windows, exposing parent is asking for troubles, we do not want to expose parent of such shells. We could set window.getParent to nil on the top multishell element.

if bShowMenu then
tProcess.window = window.create(parentTerm, 1, 2, w, h - 1, false)
else
tProcess.window = window.create(parentTerm, 1, 1, w, h, false)
end

tProcess.window.getParent = nil

2. Getting native monitor/term:

This does not solve the issue of allowing us to figure out which monitor/term we are using if it is behind multishell. While it is still useful for calculating positions, a method window:getNative() which would go as deep into parent as possible could be useful.

3. Checking if getParent exists:

It is required to check if getParent exists before calling it, which can lead to two problems:

  • could result in a bit weird/non-clean code for the end user
  • code-incompatibility for older computer-craft versions, hard to detect it due checking if .getParent exists
    Example:
function getWindowRoot(window)
  local parent = window
  while parent.getParent do
    parent = parent.getParent()
  end
  return parent
end

Mainly there are 2 issues:

  1. Parent may or may not be the monitor/native term or a multishell window, end user may assume parent is a monitor/term without considering multishells.
  2. Using this code with outdated computercraft mod - it will not throw any error whatsoever! Dangerous!

Adding {monitor/term/window}:getNative() and window:getParent()

Simply adding getParent to window can lead to other problems as mentioned above, a possibly cleaner solution could be to have getNative() - grab the monitor or term.

getParent() - used for grabbing relative positions, implemented on window
getNative() - used for figuring out where the window lives, implemented on term/monitor/window

window.getNative() -> getParent() until parent does not exist, then call getNative()
window.getParent() -> parent window/term/monitor

Possibly such methods too:

term.getNative() -> itself (term)
monitor.getNative() -> itself (monitor)

multishell would have to use this, which would still allow grabbing the native term/monitor and should work with nested multishells:

tProcess.window.getParent = nil
tProcess.window.getNative = function ()
  if parentTerm.getNative then parentTerm.getNative()
  return parentTerm
end

Note - we set getParent to nil because we always want to return parent if getParent function exists.

Okay! This solves two problems:

  1. A way to figure out if you're on a terminal or (which) monitor with term.native() == term.current().getNative()
function X.pullClick(window)
  local native = window.getNative()
  local winType = (term.native() == native) and "term" or "monitor"
  if winType == "term" then
    os.pullEvent("mouse_click")
  elseif winType == "monitor" then
    os.pullEvent("monitor_touch")
  end
  -- more code --
end
  1. Figuring out almost absolute position:
function getAbsoluteWindowPos(window)
  local x, y = 1,1
  while parent.getParent do
    local rx, ry = parent.getPosition()
    x = x + rx - 1
    y = y + ry - 1
    parent = parent.getParent() -- !! --
  end
  return x, y
end

Important consideration - term.native() already exists. This may be confusing! A different name such as getRoot(), getBaseTerm() should be considered instead.

I also considered if getParent() should exist in monitor/term, I believe that getParent could be confusing in documentation.

Otherwise, I think those two additions solve a two problems in the cleanest possible way.

TLDR

Add:

  • window:getParent()
  • window:getNative()

Consider:

  • term:getNative()
  • monitor:getNative()

Add window:getParent() for relative positioning (stops at multishell boundary if multishell sets it to nil, which is expected).
Add window:getRoot() to reliably get the actual base term or monitor, bypassing multishell (implemented by storing the root reference at creation).

This gives users tools for both relative coordinate math within an environment and identifying the true underlying display hardware.

commented

Oh yeah, *:getNative() is confusing, *:getBaseTerm() may be better. If this feature suggestion is good, I may do a PR.

I am still unsure if getNative should be added outside window, yet it would make it more universal if it was added.

Honestly, it may be unnecessary. End user could track the parent themselves.

commented

So the real thing you want is not getParent, but get the relative click position on a window?

I'd suggest you checkout some existing frameworks such as basalt

commented

Thanks for the report! I think my comment in #1561 stands here too — you don't actually want the absolute position of the window. You want the position relative to some particular frame of reference. You're much better off building abstractions on-top of the window API to allow this, rather than trying to put it in the window API directly.

commented

Thanks, thought .getParent() could be really useful, yet it wouldn't be a totally clean solution, especially since multishells exist. Totally get it why we shouldn't do it!