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

Feat/rendering multiplayer #33

Merged
merged 16 commits into from Mar 28, 2024
Merged

Feat/rendering multiplayer #33

merged 16 commits into from Mar 28, 2024

Conversation

Junyi00
Copy link
Contributor

@Junyi00 Junyi00 commented Mar 27, 2024

Local Multiplayer Rendering

Closes #23

Changes

  • Add LayoutUtils for creating layout for player views
  • GameScene now creates a camera that tracks the player as it moves
  • Update GameBridge to synchronise player entities to GameScene
  • Refactor the logic of updating overlays

Preview

local_multiplayer

@Junyi00 Junyi00 self-assigned this Mar 27, 2024
@Junyi00 Junyi00 marked this pull request as ready for review March 27, 2024 08:26
Copy link
Contributor

@jasonqiu212 jasonqiu212 left a comment

Choose a reason for hiding this comment

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

Minor comments, but the rest LGTM!

I think this is a really elegant solution, that required a lot of understanding of MetalKit and SpriteKit :)

guard let player = self.player else {
return
}
self.position = player.position
Copy link
Contributor

Choose a reason for hiding this comment

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

Should the camera only follow the x position of the player?

Copy link
Contributor

Choose a reason for hiding this comment

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

By doing so, we need to initialize y position of camera to half of the height of the scene.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I noticed it was previously only tracking the x position, but the player may jump up to higher platforms and exit the height of what the view can see...

Copy link
Contributor

Choose a reason for hiding this comment

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

Hmm that's true. My rationale for fixing the camera's y position was because if the camera track's the player's y position as well, then we may run out of the background image when going up. I guess it can be said that levels should not be limited in height (maybe can have a level that's focused in the sky or something), but with the lack of assets, would it be better to just cap how high the player can go?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Maybe we resolve this after we have a complete level ready? We can test then what feels most normal/natural. We can create an issue in the meantime

Copy link
Contributor

Choose a reason for hiding this comment

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

okay that sounds good!

Copy link
Contributor

@jasonqiu212 jasonqiu212 left a comment

Choose a reason for hiding this comment

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

LGTM!

@Junyi00 Junyi00 merged commit ba6c8ee into main Mar 28, 2024
1 check failed
@Junyi00 Junyi00 deleted the feat/rendering-multiplayer branch March 28, 2024 18:26
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.

Split-Screen Rendering
2 participants