hajimehoshi / ebiten

Ebitengine - A dead simple 2D game engine for Go
https://ebitengine.org
Apache License 2.0
11.02k stars 660 forks source link

ebiten: `AppendGamepadIDs` still returned two IDs even after disconnecting one when connecting two PS4 gamepads #3046

Closed Palessan closed 3 months ago

Palessan commented 3 months ago

Ebitengine Version

v2.7.7

Operating System

Go Version (go version)

1.22.5

What steps will reproduce the problem?

Connect 2 gamepads, and then disconnect the first connected. Then connect it again. I used PS4 controllers to test

What is the expected result?

Gamepad 1 will take control of the other player again.

What happens instead?

Both players will then be controlled by gamepad 2. The issue starts when disconnecting controller 1. If we disconnect controller 2 then no issue. This happens since ebiten.AppendGamepadIDs(connectedControllers) still returns [0,1] even after disconnecting the first controller

Anything else you feel useful to add?

You can test it with:

package main

// https://github.com/hajimehoshi/ebiten/issues/3046
// Connect 2 gamepads, and then disconnect the first connected. then connect it again.
// Both players will then be controlled by the second gamepad #3046

import (
    "fmt"
    "image/color"
    "log"
    "time"

    "github.com/hajimehoshi/ebiten/v2"
    "github.com/hajimehoshi/ebiten/v2/vector"
)

type Game struct {
    lastPrintTime time.Time
}
type Vector2 struct {
    X, Y float64
}

var (
    connectedControllers []ebiten.GamepadID
    playersPos           = []Vector2{{50, 50}, {200, 200}}
    DarkGreen            = color.RGBA{0, 117, 44, 255}
    Blue                 = color.RGBA{0, 121, 241, 255}
    Red                  = color.RGBA{230, 41, 55, 255}
)

func (g *Game) Update() error {
    connectedControllers = connectedControllers[:0]
    connectedControllers = ebiten.AppendGamepadIDs(connectedControllers)

    for i, gpId := range connectedControllers {
        axisX := ebiten.StandardGamepadAxisValue(gpId, ebiten.StandardGamepadAxisLeftStickHorizontal)
        if !(axisX > 0.1 || axisX < -0.1) {
            axisX = 0
        }
        axisY := ebiten.StandardGamepadAxisValue(gpId, ebiten.StandardGamepadAxisLeftStickVertical)
        if !(axisY > 0.1 || axisY < -0.1) {
            axisY = 0
        }
        playersPos[i].X += axisX * 5
        playersPos[i].Y += axisY * 5
    }

    return nil
}

func (g *Game) Draw(screen *ebiten.Image) {
    screen.Fill(DarkGreen)
    for i := range connectedControllers {
        vector.StrokeRect(screen, float32(playersPos[i].X), float32(playersPos[i].Y),
            float32(20), float32(20), 3, Blue, false)
    }
    // Print connected controllers every 3 seconds
    if time.Since(g.lastPrintTime) >= 3*time.Second {
        fmt.Println(connectedControllers)
        if len(connectedControllers) == 1 {
            fmt.Println(&connectedControllers[0])
            // fmt.Println(ebiten.GamepadSDLID(connectedControllers[0]))
        }
        if len(connectedControllers) == 2 {
            fmt.Println(&connectedControllers[0])
            // fmt.Println(ebiten.GamepadSDLID(connectedControllers[0]))
            fmt.Println(&connectedControllers[1])
            // fmt.Println(ebiten.GamepadSDLID(connectedControllers[1]))
        }
        g.lastPrintTime = time.Now()
    }
}

func (g *Game) Layout(outsideWidth, outsideHeight int) (int, int) {
    return 640, 480
}

func main() {
    ebiten.SetWindowSize(640, 480)
    ebiten.SetWindowTitle("2 Players with 2 Joysticks")
    game := &Game{
        // Set lastPrintTime to 3 seconds ago to print immediately on start
        lastPrintTime: time.Now().Add(-3 * time.Second),
    }
    if err := ebiten.RunGame(game); err != nil {
        log.Fatal(err)
    }
}
hajimehoshi commented 3 months ago

Thanks.

Ebitengine Version 2.22

We don't have the version 2.22. Which version did you test?

hajimehoshi commented 3 months ago

I couldn't reproduce the issue.

Are you really sure [0, 1] is returned instead of [1] even after disconnecting the first gamepad?

package main

import (
    "image/color"
    "log"

    "github.com/hajimehoshi/ebiten/v2"
    "github.com/hajimehoshi/ebiten/v2/vector"
)

type Game struct {
}

func (g *Game) Update() error {
    fmt.Println(ebiten.AppendGamepadIDs(nil))
    return nil
}

func (g *Game) Draw(screen *ebiten.Image) {
}

func (g *Game) Layout(outsideWidth, outsideHeight int) (int, int) {
    return 640, 480
}

func main() {
    game := &Game{}
    if err := ebiten.RunGame(game); err != nil {
        log.Fatal(err)
    }
}
hajimehoshi commented 3 months ago

I think I could reproduce the issue by using the same PS4 gamepads w/ Ebitengine 3a6aaac5ac9b3009b232de44602a138c4cd9c42a. This cannot be reproduced on macOS.

hajimehoshi commented 3 months ago

Apparently the callback for EnumDevices iterates wrong IDs

// Connect the first PS4 controller
--enum start--
{863838F0-0360-11EF-8001-444553540000}
--enum end--
// Connect the second PS4 controller
--enum start--
{863838F0-0360-11EF-8001-444553540000}
{E13EA290-4693-11EF-8002-444553540000}
--enum end--
// Disconnect the first PS4 controller
--enum start--
{863838F0-0360-11EF-8001-444553540000} // This iterates the first connection..?
--enum end--
Palessan commented 3 months ago

Thanks.

Ebitengine Version 2.22

We don't have the version 2.22. Which version did you test?

Sorry, it is the latest, version v2.7.7

Palessan commented 3 months ago

It is also reproducible with inpututil.AppendJustConnectedGamepadIDs(connectedControllers)

package main

// https://github.com/hajimehoshi/ebiten/issues/3046
// Connect 2 gamepads, and then disconnect the first connected. then connect it again.
// Both players will then be controlled by the second gamepad #3046

import (
    "fmt"
    "image/color"
    "log"
    "time"

    "github.com/hajimehoshi/ebiten/v2"
    "github.com/hajimehoshi/ebiten/v2/inpututil"
    "github.com/hajimehoshi/ebiten/v2/vector"
)

type Game struct {
    lastPrintTime time.Time
}
type Vector2 struct {
    X, Y float64
}

var (
    connectedControllers []ebiten.GamepadID
    playersPos           = []Vector2{{50, 50}, {200, 200}}
    DarkGreen            = color.RGBA{0, 117, 44, 255}
    Blue                 = color.RGBA{0, 121, 241, 255}
    Red                  = color.RGBA{230, 41, 55, 255}
)

func (g *Game) Update() error {

    connectedControllers = inpututil.AppendJustConnectedGamepadIDs(connectedControllers)
    if inpututil.IsGamepadJustDisconnected(0) {
        connectedControllers = connectedControllers[1:]
    }
    if inpututil.IsGamepadJustDisconnected(1) {
        connectedControllers = append(connectedControllers[:1], connectedControllers[2:]...)
    }

    // for i := 0; i < 5; i++ {
    //  if inpututil.IsGamepadJustDisconnected(ebiten.GamepadID(i)) {
    //      connectedControllers = append(connectedControllers[:i], connectedControllers[i+1:]...)
    //  }
    // }

    for i, gpId := range connectedControllers {
        axisX := ebiten.StandardGamepadAxisValue(gpId, ebiten.StandardGamepadAxisLeftStickHorizontal)
        if !(axisX > 0.2 || axisX < -0.2) {
            axisX = 0
        }
        axisY := ebiten.StandardGamepadAxisValue(gpId, ebiten.StandardGamepadAxisLeftStickVertical)
        if !(axisY > 0.2 || axisY < -0.2) {
            axisY = 0
        }
        playersPos[i].X += axisX * 5
        playersPos[i].Y += axisY * 5
    }

    return nil
}

func (g *Game) Draw(screen *ebiten.Image) {
    screen.Fill(DarkGreen)
    for i := range connectedControllers {
        vector.StrokeRect(screen, float32(playersPos[i].X), float32(playersPos[i].Y),
            float32(20), float32(20), 3, Blue, false)
    }

    // Print connected controllers every 5 seconds
    if time.Since(g.lastPrintTime) >= 3*time.Second {
        fmt.Println(connectedControllers)
        if len(connectedControllers) == 1 {
            fmt.Println(&connectedControllers[0])
            // fmt.Println(ebiten.GamepadSDLID(connectedControllers[0]))
        }
        if len(connectedControllers) == 2 {
            fmt.Println(&connectedControllers[0])
            // fmt.Println(ebiten.GamepadSDLID(connectedControllers[0]))
            fmt.Println(&connectedControllers[1])
            // fmt.Println(ebiten.GamepadSDLID(connectedControllers[1]))
        }
        g.lastPrintTime = time.Now()
    }
}

func (g *Game) Layout(outsideWidth, outsideHeight int) (int, int) {
    return 640, 480
}

func main() {
    ebiten.SetWindowSize(640, 480)
    ebiten.SetWindowTitle("2 Players with 2 Joysticks")
    game := &Game{
        // Set lastPrintTime to 5 seconds ago to print immediately on start
        lastPrintTime: time.Now().Add(-3 * time.Second),
    }
    if err := ebiten.RunGame(game); err != nil {
        log.Fatal(err)
    }
}
hajimehoshi commented 3 months ago

Please use three backquotes to make the code readable in the comment. Thanks,

hajimehoshi commented 3 months ago

In the worst case, this can happen even without disconnecting one.

  1. Connect a PS4 controller
  2. Connect another PS4 controller. AppendGamepadIDs returns two IDs, but the two IDs are assinged for the same physical first game pad.

I could reproduce on Parallels. I guess this is a bug in Direct Input 8 and/or Parallels and I have no idea how to fix this... 🤔

EDIT: SDL considers 'paths' when adding a new gamepad. This might be a hint https://github.com/libsdl-org/SDL/blob/e1aa99573270eac053b428cc899eebf499b44eca/src/joystick/windows/SDL_dinputjoystick.c#L469-L499

Palessan commented 3 months ago

tried it in reylib-go and i could not replicate it there. There we just have l.IsGamepadAvailable(gpId)

package main

import (
    "fmt"
    "time"

    rl "github.com/gen2brain/raylib-go/raylib"
)

type Vector2 struct {
    X, Y float32
}

var (
    allGamepads = []int32{0, 1, 2, 3}
    gamepads    = []int32{}
    playersPos  = []Vector2{{50, 50}, {200, 200}}
    DarkGreen   = rl.NewColor(0, 117, 44, 255)
    Blue        = rl.NewColor(0, 121, 241, 255)
    lastPrintTime = time.Now().Add(-3 * time.Second)

)

func draw() {
    gamepads := gamepads[:0]
    for _, gpId := range allGamepads {
        if rl.IsGamepadAvailable(gpId) {
            gamepads = append(gamepads, gpId)
        }
    }

    for i, gpId := range gamepads {
        if i >= len(playersPos) {
            break
        }
        axisX := rl.GetGamepadAxisMovement(gpId, rl.GamepadAxisLeftX)
        if !(axisX > 0.1 || axisX < -0.1) {
            axisX = 0
        }
        axisY := rl.GetGamepadAxisMovement(gpId, rl.GamepadAxisLeftY)
        if !(axisY > 0.1 || axisY < -0.1) {
            axisY = 0
        }
        playersPos[i].X += axisX * 5
        playersPos[i].Y += axisY * 5
    }

    if time.Since(lastPrintTime) >= 3*time.Second {
        fmt.Println(gamepads)
        for _, gpId := range gamepads {
            fmt.Printf("\nGamepad %d: %s", gpId, rl.GetGamepadName(gpId))
        }
        lastPrintTime = time.Now()
    }

    rl.BeginDrawing()
    rl.ClearBackground(DarkGreen)
    for i := range gamepads {
        rl.DrawRectangle(int32(playersPos[i].X), int32(playersPos[i].Y), 20, 20, Blue)
    }
    rl.EndDrawing()
}

func main() {
    rl.InitWindow(640, 480, "2 Players with 2 Joysticks")
    defer rl.CloseWindow()

    for !rl.WindowShouldClose() {
        draw()

    }
}
hajimehoshi commented 3 months ago

Please try e5bb046a116841a98e0d807c7a1ffd4d1816a87c 29946d037be5b22624e75edf48b8dbf3d87e7e61 (2.7) or 122877c265b2a9ce60e553bf0e8b14e1f56b0ece 7ab9382424311ee455b622658795eaaac74dd7f5 (main). Thanks,

go get github.com/hajimehoshi/ebiten/v2@29946d037be5b22624e75edf48b8dbf3d87e7e61
hajimehoshi commented 3 months ago

I filed a crash by the fix... #3047

EDIT: Fixed!

Palessan commented 3 months ago

go get github.com/hajimehoshi/ebiten/v2@29946d037be5b22624e75edf48b8dbf3d87e7e61

Tested it and it is almost fixed.

Further issue: When controller 1 is disconnected, the controllers switch control. For example when you disconnect controller 1, then square of controller 2 disappears and controller 2 controls the square 1. (its ok when disconnecting controller 2. square 2 disappears)

hajimehoshi commented 3 months ago

Thank you for confirming.

When controller 1 is disconnected, the controllers switch control. For example when you disconnect controller 1, then square of controller 2 disappears and controller 2 controls the square 1. (its ok when disconnecting controller 2. square 2 disappears)

You meant a gamepad ID by 'square'?

hajimehoshi commented 3 months ago

OK I think you meant a blue square in your program. In your program,

    for i, gpId := range connectedControllers {
        axisX := ebiten.StandardGamepadAxisValue(gpId, ebiten.StandardGamepadAxisLeftStickHorizontal)
        if !(axisX > 0.1 || axisX < -0.1) {
            axisX = 0
        }
        axisY := ebiten.StandardGamepadAxisValue(gpId, ebiten.StandardGamepadAxisLeftStickVertical)
        if !(axisY > 0.1 || axisY < -0.1) {
            axisY = 0
        }
        playersPos[i].X += axisX * 5
        playersPos[i].Y += axisY * 5
    }

playerPos's index is connectedControllers's index, not a gamepad IDs, and I think that's why. The following program should work.

package main

import (
    "fmt"
    "image/color"
    "log"
    "time"

    "github.com/hajimehoshi/ebiten/v2"
    "github.com/hajimehoshi/ebiten/v2/vector"
)

type Game struct {
    lastPrintTime time.Time
}
type Vector2 struct {
    X, Y float64
}

var (
    connectedControllers []ebiten.GamepadID
    playersPos           = map[ebiten.GamepadID]Vector2{
        0: {50, 50},
        1: {200, 200},
    } // 0, 1 is hardcoded IDs so this is not practical, but it is OK for testing.
    DarkGreen = color.RGBA{0, 117, 44, 255}
    Blue      = color.RGBA{0, 121, 241, 255}
    Red       = color.RGBA{230, 41, 55, 255}
)

func (g *Game) Update() error {
    connectedControllers = connectedControllers[:0]
    connectedControllers = ebiten.AppendGamepadIDs(connectedControllers)

    for _, gpId := range connectedControllers {
        axisX := ebiten.StandardGamepadAxisValue(gpId, ebiten.StandardGamepadAxisLeftStickHorizontal)
        if !(axisX > 0.1 || axisX < -0.1) {
            axisX = 0
        }
        axisY := ebiten.StandardGamepadAxisValue(gpId, ebiten.StandardGamepadAxisLeftStickVertical)
        if !(axisY > 0.1 || axisY < -0.1) {
            axisY = 0
        }
        pos := playersPos[gpId]
        pos.X += axisX * 5
        pos.Y += axisY * 5
        playersPos[gpId] = pos
    }

    return nil
}

func (g *Game) Draw(screen *ebiten.Image) {
    screen.Fill(DarkGreen)
    for _, id := range connectedControllers {
        vector.StrokeRect(screen, float32(playersPos[id].X), float32(playersPos[id].Y),
            float32(20), float32(20), 3, Blue, false)
    }
    // Print connected controllers every 3 seconds
    if time.Since(g.lastPrintTime) >= 3*time.Second {
        fmt.Println(connectedControllers)
        if len(connectedControllers) == 1 {
            fmt.Println(&connectedControllers[0])
            // fmt.Println(ebiten.GamepadSDLID(connectedControllers[0]))
        }
        if len(connectedControllers) == 2 {
            fmt.Println(&connectedControllers[0])
            // fmt.Println(ebiten.GamepadSDLID(connectedControllers[0]))
            fmt.Println(&connectedControllers[1])
            // fmt.Println(ebiten.GamepadSDLID(connectedControllers[1]))
        }
        g.lastPrintTime = time.Now()
    }
}

func (g *Game) Layout(outsideWidth, outsideHeight int) (int, int) {
    return 640, 480
}

func main() {
    ebiten.SetWindowSize(640, 480)
    ebiten.SetWindowTitle("2 Players with 2 Joysticks")
    game := &Game{
        // Set lastPrintTime to 3 seconds ago to print immediately on start
        lastPrintTime: time.Now().Add(-3 * time.Second),
    }
    if err := ebiten.RunGame(game); err != nil {
        log.Fatal(err)
    }
}
Palessan commented 3 months ago

Yes the above example works but in the raybin-go example it is different ids and gpids, and still works without the above fix.. There may still be an issue with wrong id returned but not 100% sure. Will test more tomorrow.

hajimehoshi commented 3 months ago

I don't know how raylib works but probably the philosophy is different. In Ebitengine, gamepad IDs matter anyway.

Thank you for reporting the issue!

Palessan commented 3 months ago

Thank you for solving it so fast! I tested a bit more and its all good! Also it does have the same functionality as raybin-go as tests are concerned, I was wrong before.

One question: Is there any performance benefit doing position update calculations under Update rather than directly inside the Draw?

hajimehoshi commented 3 months ago

Is there any performance benefit doing position update calculations under Update rather than directly inside the Draw?

There is not much performance difference, but updating logics should be done in Update. How many times Draw is called (FPS) depends on the environment (e.g. if you use a 144 Hz display, FPS will be 144 Hz), but how many times Update is called (TPS) should be always the same (60 by default).

hajimehoshi commented 3 months ago

Thank you for reporting the issue! I'll release v2.7.8 in a few days if there is no other issue.