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

only check for new instance when entering from a different area #4

Open
wants to merge 1 commit into
base: instancing
Choose a base branch
from

Conversation

ratacat
Copy link

@ratacat ratacat commented Oct 14, 2019

otherwise moving around within the instance fires off new instances

otherwise moving around within the instance fires off new instances
@@ -157,12 +157,14 @@ function getRoom(state, player, nextRoomId) {
let nextRoom = null;
if (state.AreaFactory.isInstanced(nextAreaName)) {
const instanceId = player.name;
B.sayAt(player, `Entering instanced area: ${nextAreaName}`);
if (player.area != nextAreaName ){
Copy link
Contributor

Choose a reason for hiding this comment

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

Some of the spacing here is a bit off, and it is best practice to use !== (though I am not sure this would cause issues unless the area names were undefined/null/0/''...

Copy link
Author

Choose a reason for hiding this comment

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

Whats the best way for me to edit the pull request? do I just need to submit another one?

Copy link
Contributor

Choose a reason for hiding this comment

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

Nah, you can edit it using GitHub's UI, or you can use Git to pull down the branch, edit it locally, and then push it to the same remote branch.

@seanohue
Copy link
Contributor

This is a good catch and should be merged into the instancing branch if possible.

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