Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

do-not-merge: online functionality #52

Open
wants to merge 8 commits into
base: nightly
Choose a base branch
from

Conversation

cunev
Copy link

@cunev cunev commented Nov 17, 2024

The added Socket.gd script initializes a WebSocket client (WebSocketClient) and communicates with another local websocket server process by sending JSON-formatted messages. It handles connection events, receives data, and sends specific game-related states, such as map start, menu state, and map end.

Example in a Node.Js server

Testing:

  • manually tested

image

Why

  • Allows integration with Online client and other helpers that might want live game data.
  • osu! has similar interface, and it has been proven useful for tournaments.

print("Connection Established", proto)

# Handles data received from the WebSocket
func _on_data_received():
Copy link
Author

Choose a reason for hiding this comment

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

Currently useless, will be used in the future to give ability to control game.
(maybe?)

@cunev
Copy link
Author

cunev commented Nov 17, 2024

Do not merge yet!

extends Node

# URL for the WebSocket server
export var SOCKET_URL = "ws://127.0.0.1:3000"
Copy link
Contributor

Choose a reason for hiding this comment

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

pls pick a unique port
i run test servers on port 3000

Copy link
Collaborator

Choose a reason for hiding this comment

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

What kermeet said here is valid, 3000 is used for general testing

Copy link
Collaborator

@mycpphurts mycpphurts left a comment

Choose a reason for hiding this comment

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

Commits look good, but some changes could be done 😊

extends Node

# URL for the WebSocket server
export var SOCKET_URL = "ws://127.0.0.1:3000"
Copy link
Collaborator

Choose a reason for hiding this comment

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

What kermeet said here is valid, 3000 is used for general testing

Polls the WebSocket client for updates. This keeps the connection
alive and processes incoming/outgoing packets.
"""
_wsClient.poll()
Copy link
Collaborator

Choose a reason for hiding this comment

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

Maybe it's a good idea to put the poll in the _physics_process like @fogsaturate said in private, gd3 networking is awful

@mycpphurts
Copy link
Collaborator

Didnt tick the "Requesting changes" thing but thanks git I guess...

@cunev cunev changed the title feat: add connect to websocket server if possible on game start, send… do-not-merge: add connect to websocket server if possible on game start, send… Nov 18, 2024
@fogsaturate
Copy link
Collaborator

this might seem like a dumb idea but i think it'd be cool if you added the current language selected as a websocket function just in case external programs utilize that, and also has said translation for said program

@mycpphurts mycpphurts dismissed their stale review December 8, 2024 22:04

Didnt mean to request changes and instead comment (for comments pls refer to #52 (review))

@mycpphurts
Copy link
Collaborator

Chat do we merge this :trolley

@cunev
Copy link
Author

cunev commented Jan 20, 2025

Do not merge, this is still heavily tested and being improved.
When finished I will give greenlight to merge.
Thanks!

@cunev cunev changed the title do-not-merge: add connect to websocket server if possible on game start, send… do-not-merge: online functionality Jan 20, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants