AUTO_FOLLOW_PLAYER setting requires Player object to be named "player"?

[racingcow]

It looks like the Camera class requires that a player is named "player" in order for the camera to follow properly.

In my case, I have several players, each named something other than "player". Would it make sense to fallback to searching by class name/type/group, similar to the way that creature.findPredatorPrey does?

I can send a PR if it makes sense and you'd like me to.

[collinhover]

There are several instances where the library assumes there is only a single player entity and it is named "player", which is the default in the ig.Player abstract. You're working with a special case that breaks this assumption, but I admit this doesn't seem like such an uncommon case. If you have a good idea of how this can be handled, I think it would better match the library's design goals than the current setup. One issue I can see relating to the camera auto follow is choosing which of several players to follow, so there would need to be some sort of prioritization.

[racingcow]

How about (at least at first) adding a simple getActivePlayer utility method that would determine the active player? Perhaps by searching in the order below. In cases where more than one entity is found, the first one is chosen by default. If more complex logic is needed, users could override the current behavior.

Search priorities...

1. the entity with name = "player"
2. the first entity with group flag GROUP = PLAYER set
3. the first entity with type flag TYPE = PLAYER set
4. the first entity with class = ig.Player

Here are the places I found where it was assumed that only one Player named "player" existed...

• camera.js
• tutorial.js (L343)
• tutorial.js (L431)
• tutorial.js (L683)
• game.js
• conversation.js (L423)
• conversation.js (L499)
• ui-tracker.js

I sent PR 55 for this. I ran both my game and the jumpnrun demo against this new code, and both seemed to work in the latest Firefox Nightly, Chrome Canary, and Internet Explorer. Let me know if you see anything that I missed, or that needs modification.

[collinhover]

This seems like a good solution, and it is consistent with the way the creature abstract aquires a prey or predator. However, having the getActivePlayer method in the ig.Player abstract may be prone to looping requirements, because anything that needs to reference the player will need to require it, which also disqualifies it as a requirement for the player itself. A common pattern in ImpactJS and Impact++ is to assume ig.game is present, as it is set before anything else, so how about moving the getActivePlayer into ig.GameExtended? This also groups the "get some kind of entity" methods together as well.

[collinhover]

Also, any reason why it is named getActivePlayer as opposed to just getPlayer?

[collinhover]

One other suggestion: I'd change the order from (1) name, (2) group, (3), type, (4) class

to

(1) name, (2) class, (3) type, and (4) group

In fact, I might even remove group from the search entirely. The reason being, group is a very general collection of entities that do not collide or check against each other. So a group of PLAYER can and is commonly assigned to things such as projectiles that the player has created, so that they do not collide and hurt the player. Types and classes, however, are safe as they are assigned only to entities that are actually players (types and groups are different, even if some are named similarly).

[racingcow]

That makes sense about requirements looping and grouping.

As for the name, I was calling it getActivePlayer to imply the possibility of having multiple player objects, but only one being active at a time. Perhaps something like a turn-based board game with multiple people playing at the same time. In cases like those, I was thinking there could be multiple 'Player' objects, but only one would be active (switching on each "turn"). Users of Impact++ could override getActivePlayer to provide logic on how to select the correct one.

I don't mind having it called just getPlayer, however, and have made that change as well.

Let me know if you see anything else that should be modified.

[collinhover]

Looking good! I'll review and merge later tonight.

[collinhover]

Sorry for the delay, been busy trying to make a kitchen sink demo of the library for onGameStart conference. I just noticed that the pull request is for the master branch. Can you close the current PR and create a new one with the dev branch please?

[racingcow]

No problem, @collinhover, and sorry about marking it for the master. I think I've got it resubmitted for dev, now.

[collinhover]

Merged in! Cheers!