Skip to content

Enable swapping ships when in lobby - #68

Merged
JNProtzman merged 1 commit into
masterfrom
handle-ship-change
Aug 23, 2026
Merged

Enable swapping ships when in lobby#68
JNProtzman merged 1 commit into
masterfrom
handle-ship-change

Conversation

@JNProtzman

Copy link
Copy Markdown
Collaborator

This is useless since we have one ship - but I did test adding a second ship and switching between them. And it does prevent a client crash if someone clicks the button.

@JNProtzman
JNProtzman requested a review from dcrodman August 21, 2026 06:16
Comment thread internal/character_server.go Outdated
// IP address and port of the ship server to which the client will connect after
// disconnecting from this server.
func (s *CharacterServer) handleShipSelection(ctx context.Context, c *Client, menuSelectionPkt *commands.MenuSelection) error {
func HandleShipSelection(ctx context.Context, c *Client, menuSelectionPkt *commands.MenuSelection) error {

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

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

Mind throwing a comment on here pointing out that this is used by both character and ship since it's the same menu but shared across connections? It just strays from the convention I have around the handle* methods being per-server, which is fine but worth noting

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

Rather than add a comment, I've copied the relevant function into the ship server. I do wonder if we should DRY that up but maybe it's not worth it for now?

@dcrodman dcrodman left a comment

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

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

It actually used to support this and I dropped it because so much else needs to change if Archon eventually supports other ships. But sure, doesn't hurt to have it

@JNProtzman
JNProtzman force-pushed the handle-ship-change branch 2 times, most recently from 595a0d2 to 5bd3aae Compare August 22, 2026 16:27
@JNProtzman
JNProtzman requested a review from dcrodman August 22, 2026 16:30
@JNProtzman
JNProtzman merged commit a31b00b into master Aug 23, 2026
2 checks passed
@JNProtzman
JNProtzman deleted the handle-ship-change branch August 23, 2026 05:12
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.

2 participants