Hexworks / zircon

Zircon is an extensible and user-friendly, multiplatform tile engine.
https://hexworks.org/projects/zircon/
Apache License 2.0
753 stars 137 forks source link

MouseEvents should contain the relative position inside its component #335

Open Baret opened 4 years ago

Baret commented 4 years ago

Describe the bug Currently when adding a mouse listener to a component you can get the position of the click (or movement) from it. But this position is absolute. This means it is independent of the component's position. This becomes a problem when you add a handler that has no idea of where the component is located on the screen.

To Reproduce

import org.hexworks.zircon.api.ColorThemes
import org.hexworks.zircon.api.ComponentDecorations
import org.hexworks.zircon.api.Components
import org.hexworks.zircon.api.SwingApplications
import org.hexworks.zircon.api.application.AppConfig
import org.hexworks.zircon.api.data.Position
import org.hexworks.zircon.api.data.Size
import org.hexworks.zircon.api.extensions.toScreen
import org.hexworks.zircon.api.uievent.MouseEventType
import org.hexworks.zircon.api.uievent.Processed

private val WINDOW_SIZE = Size.create(60, 30)

private val panel = Components
        .panel()
        .withPosition(8, 5)
        .withSize(42, 20)
        .withDecorations(ComponentDecorations.box())
        .build()

private val clickLabel = Components
        .label()
        .withPosition(Position.zero())
        .withSize(WINDOW_SIZE.width, 1)
        .build()

private val clickLabelAbsolute = Components
        .label()
        .withPosition(Position.bottomLeftOf(clickLabel))
        .withSize(WINDOW_SIZE.width, 1)
        .build()

private val infoLabel = Components
        .label()
        .withPosition(Position.bottomLeftOf(clickLabelAbsolute))
        .withSize(WINDOW_SIZE.width - 1, 1)
        .build()

fun main() {
    val screen = SwingApplications
            .startTileGrid(AppConfig
                    .newBuilder()
                    .withSize(WINDOW_SIZE)
                    .withTitle("Mouse events")
                    .build())
            .toScreen()
    with(screen) {
        addComponents(
                panel,
                clickLabel,
                clickLabelAbsolute,
                infoLabel
        )
        theme = ColorThemes.war()
        display()
    }

    panel.handleMouseEvents(MouseEventType.MOUSE_CLICKED) { event, phase ->
        // Imagine that panel.absolutePosition is unknown here...
        val absolutePosition = event.position
        val position = absolutePosition - panel.absolutePosition
        clickLabel.textProperty.updateValue("You clicked       ${position.x},${position.y} in the panel")
        clickLabelAbsolute.textProperty.updateValue("Event position is ${absolutePosition.x},${absolutePosition.y}")
        Processed
    }

    infoLabel.textProperty.updateValue("Abs. position of panel: ${panel.absolutePosition}")

}

Expected behavior When handling a mouse event I would expect to either get the event.position inside the component that handles the event. Or have two fields in the event: event.absolutePosition and event.position.

adam-arold commented 4 years ago

The mouse listener works with any Zircon object, not just Components, that's why it doesn't have a relative position. You bring up a valid point though. Can you think of a way we could augment MouseEvent without breaking the API? I'm gonna add this to the backlog.

Baret commented 4 years ago

Without deeper checks of where mouse events are created and consumed...

Without breaking the API? Probably by creating a new class that gets new special treatment (new handleXEvent() methods and so on):

/**
 * A mouse event triggered by a [Component].
 */
data class MouseComponentEvent(
        override val type: MouseEventType,
        val button: Int,
        /**
         * Position of this event relative to the top left corner of the tile grid/screen.
         */
        val position: Position,
        /**
         * The [Component] that triggered this event.
         */
        val source: Component
) : UIEvent {
    /**
     * The position of this event inside the [source] component. This position is relative to
     * the top left corner of [source]'s absolutePosition.
     *
     * @see Component.absolutePosition
     */
    val positionInSource: Position
        get() = position - source.absolutePosition
}

fun MouseEvent.toMouseComponentEvent(source: Component) =
        MouseComponentEvent(
                type,
                button,
                position,
                source
        )

But with breaking the API? Just a quick mockup of which I acutally don't know if it woud break the API because of the default value:

data class MouseEvent<T: Any>(
        override val type: MouseEventType,
        val button: Int,
        val position: Position,
        val source: T? = null
) : UIEvent

val MouseEvent<Component>.positionInComponent: Position
    get() = position - (source?.absolutePosition ?: Position.zero())
adam-arold commented 4 years ago

This looks like a good idea, I dig it. I'm gonna take a look after the next release if this can be implemented. In the meantime, you are welcome to give it a try if you're interested.

Baret commented 4 years ago

Damn, when adding a generic type parameter to MouseEvent we would need to add it in almost every place. Usually this would probably simply result in changing MouseEvent to MouseEvent<*>. I will see what else changes with that try...

adam-arold commented 4 years ago

Oh, yeah...generics is cancer in this case...once I refactored Component to be generic just to undo it after I was done because it was so horrifying...