Skip to content
This repository has been archived by the owner on Oct 20, 2021. It is now read-only.

Upgrade entities #191

Merged
merged 5 commits into from
Jun 7, 2019
Merged

Upgrade entities #191

merged 5 commits into from
Jun 7, 2019

Conversation

zeroZshadow
Copy link
Contributor

Description

Upgrade the FPS to use the new Entities API's.

Tests

Ran locally in every scene.
Cloud deployment is currently launching

Documentation

Changelog updated.

Remove BlittableBool
Fix bug where stopping the game threw an error about removing the authoritative player entity.
@@ -45,7 +46,10 @@ protected override void OnUpdate()
}
else
{
SessionStatus = sessionGroup.GetComponentDataArray<Session.Component>()[0].Status;
Entities.With(sessionGroup).ForEach((ref Session.Component session) =>
Copy link
Contributor

Choose a reason for hiding this comment

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

is there any chance that we have more than 1 entity matching this query?

Copy link
Contributor

Choose a reason for hiding this comment

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

if you misconfigure it ><
but you should always have at most one session entity in your deployment.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@jessicafalk I think no?

Copy link
Contributor

@jessicafalk jessicafalk Jun 6, 2019

Choose a reason for hiding this comment

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

as mentioned: our snapshot always only contains at most one entity. A user can of course mess around with it and change it, but we assume you only have it at most once

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Is it oke by you to leave it like this?

Copy link
Contributor

Choose a reason for hiding this comment

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

it's not nice, but ok to leave it like this for now. It's not a big issue

Copy link
Contributor

@paulbalaji paulbalaji left a comment

Choose a reason for hiding this comment

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

generally lgtm

@zeroZshadow zeroZshadow merged commit cc9305b into develop Jun 7, 2019
@zeroZshadow zeroZshadow deleted the feature/upgrade-entities branch June 7, 2019 11:53
@zeroZshadow zeroZshadow changed the title Feature/upgrade entities Upgrade entities Jun 10, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants