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

Use custom commands instead of observers for spawning #242

Merged
merged 10 commits into from
Aug 8, 2024

Conversation

janhohenheim
Copy link
Member

Part of #203
Alternative to #231
Does not help with #223.

@janhohenheim janhohenheim requested a review from benfrankel August 7, 2024 21:08
@janhohenheim
Copy link
Member Author

@alice-i-cecile is this what you had in mind for the simple but limited spawning? Or would you prefer if I used a struct SpawnPlayer that the user then manually adds?

src/demo/level.rs Outdated Show resolved Hide resolved
src/demo/player.rs Outdated Show resolved Hide resolved
Comment on lines 34 to 35
/// Command to spawn the player character.
/// We can add fields to this struct if we need data for spawning.
Copy link
Collaborator

Choose a reason for hiding this comment

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

Suggested change
/// Command to spawn the player character.
/// We can add fields to this struct if we need data for spawning.
/// A command to spawn the player character.

I don't like the second line here personally. There are a lot of things the user could do to expand on the template, and we're not going to list them all. At the very least this doesn't make sense as a doc comment.

Copy link
Member Author

@janhohenheim janhohenheim Aug 8, 2024

Choose a reason for hiding this comment

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

We do however mention things that have a very high probability of coming up and are low-hanging expansions.
I want to keep this comment somewhere in the file, preferably around the struct.
Moved the comment into the struct: d2c59b6 (#242)

@janhohenheim janhohenheim requested a review from benfrankel August 8, 2024 19:32
Copy link
Collaborator

@benfrankel benfrankel left a comment

Choose a reason for hiding this comment

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

Approved. I think spawn_level should spawn a parent level entity with a Level marker component ideally, but this PR doesn't have to change the current behavior.

@janhohenheim janhohenheim enabled auto-merge (squash) August 8, 2024 19:36
@benfrankel benfrankel disabled auto-merge August 8, 2024 19:44
@janhohenheim janhohenheim enabled auto-merge (squash) August 8, 2024 19:50
@janhohenheim janhohenheim disabled auto-merge August 8, 2024 19:55
@janhohenheim janhohenheim enabled auto-merge (squash) August 8, 2024 20:01
@janhohenheim janhohenheim merged commit 3ec03f0 into main Aug 8, 2024
4 checks passed
@janhohenheim janhohenheim deleted the spawn-command branch August 8, 2024 20:11
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