feat: Prevent players from joining the larger team

[ahv15]

Contains

If a player tries to join the larger team, the player is notified through the chat box to join the other team. Requires https://github.com/Terasology/LightAndShadowResources/pull/63 to test this.

How to test

In multiplayer try to join the larger team.

Outstanding before merging

• [x] Prevent players from joining the larger team.

[jdrueckert]

Improve LaS team balancing

[ahv15]

@skaldarnar If these changes seem reasonable I will send in a separate PR to LightAndShadowResources too.

[ahv15]

Ok will make those changes (didn't see the problem with the disconnecting players). Is there any particular scenario where it would be preferred to use prefabs directly? Also are there any plans to move game state information to this entity. I was going to add some more configurable items (friendly fire, goal score etc) once this PR is finalized before creating a screen for configuration but due to my final exams next week I probably can't spend too much time working on it.

[skaldarnar]

Is there any particular scenario where it would be preferred to use prefabs directly?

Not sure whether I understand the question correctly. A prefab is just a blue print for constructing an entity and pre-fill it with some default values. Given that, I'd think that using a prefab for the default configuration for Light and Shadow would be a good idea.

[ahv15]

Is there any particular scenario where it would be preferred to use prefabs directly?

Not sure whether I understand the question correctly. A prefab is just a blue print for constructing an entity and pre-fill it with some default values. Given that, I'd think that using a prefab for the default configuration for Light and Shadow would be a good idea.

@skaldarnar I had this doubt because @jdrueckert suggested using a gameplay entity rather than a gameplay prefab for configuration. Just wanted to know when which is preferred.

[jdrueckert]

@skaldarnar I had this doubt because @jdrueckert suggested using a gameplay entity rather than a gameplay prefab for configuration. Just wanted to know when which is preferred.

@ahv15 I was targeting exactly the

A prefab is just a blue print for constructing an entity and pre-fill it with some default values.

aspect. In-game, we want to use the entity that was pre-filled using a prefab instead of directly accessing the prefab.

Will have another look hopefully tomorrow, today still busy with core efforts :see_no_evil:

[ahv15]

We're getting there!

I would like to make one proposal for setGameEntity() and how we use it. The typical use case in your code is to do the following:

        if (gameEntity == null || gameEntity == EntityRef.NULL) {
            setGameEntity();
        }
        // access gameEntity here being sure it is initialized

This makes use of the gameEntity being a member of the system, but otherwise bears potential errors like forgetting to check for initialization and such.

If gameEntity would not be a local member you usually would call getGameEntity() of some other class/system, and you would rely on it being present (and created if needed).

So, why not apply this pattern here as well? On the caller site, we would have something like this. Note that there are no checks on caller side here - all we express is that we want to have the game entity to work with.

        EntityRef gameEntity = getGameEntity();

Within the getter, we combine both the check for initialization and the lazy initialization/caching. A clear benefit is that we only care about this in one location, instead of in many. Plus, it's less code duplication, and will also make it simpler to extract "game entity handling" into a dedicated system (I don't think we should keep the logic of creating the entity in a system like TeleporterSystem). The implementation could look like this:

    private EntityRef getGlobalGameEntity() {
        if (globalGameEntity == null || globalGameEntity == EntityRef.NULL) {
            ArrayList<EntityRef> gameEntities =
                    Lists.newArrayList(entityManager.getEntitiesWith(LASConfigComponent.class));

            if (gameEntities.isEmpty()) {
                globalGameEntity = entityManager.create("LightAndShadow:gameEntity");
            } else if (gameEntities.size() == 1) {
                globalGameEntity = gameEntities.get(0);
            } else {
                //TODO: crash hard!
                throw new RuntimeException("Multiple game state entities available.");
            }
        }

        return globalGameEntity;
    }

Note, that throwing an exception here currently does not crash the game. I've started a discussion on Discord about this. Not sure whether we can/should mitigate this issue here in the system (e.g., choose the first entity, or always the one with lowest entity id or something like that).

Although the latest commit allows the player to join a team, it creates a new entity when a saved game is used so the underlying issue still causes a problem as the initially created entity is not detected using getEntitiesWith() (when reopening a saved game). So if maxTeamSizeDifference is modified and the game is loaded at a later point of time, then rather than getting the previously configured maxTeamSizeDifference the default value is used instead.