Skip to content

Conversation

@srmeier
Copy link
Contributor

@srmeier srmeier commented May 18, 2025

Description

Addressing issue #388 by swapping around the addRoomToMemory and SaveRoomTemplate function calls in the CreateZone function.

Changes

Provide a bullet point list of noteworthy changes in this Pull Request:

  • Calling addRoomToMemory before SaveRoomTemplate in CreateZone.

@srmeier srmeier requested a review from Volte6 as a code owner May 18, 2025 18:32
@Jasrags Jasrags requested a review from Copilot May 19, 2025 16:40
@Jasrags Jasrags added the bug Something isn't working label May 19, 2025
Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull Request Overview

This PR addresses issue #388 by reordering function calls in CreateZone to avoid potential null pointer dereferences.

  • Calls addRoomToMemory before SaveRoomTemplate.
  • Removes the duplicate addRoomToMemory call after saving the room template.

return 0, err
}

addRoomToMemory(newRoom)
Copy link

Copilot AI May 19, 2025

Choose a reason for hiding this comment

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

The error returned from addRoomToMemory is not being checked, which may obscure potential issues when updating the room cache. Consider capturing and handling this error to ensure robust function behavior.

Suggested change
addRoomToMemory(newRoom)
if err := addRoomToMemory(newRoom); err != nil {
return 0, fmt.Errorf("failed to add room to memory: %w", err)
}

Copilot uses AI. Check for mistakes.
Copy link
Member

@Volte6 Volte6 left a comment

Choose a reason for hiding this comment

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

Have not tested but looks fine to me.

@Volte6 Volte6 merged commit 4ca8a5a into GoMudEngine:master May 19, 2025
2 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bug Something isn't working

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants