Skip to content

Commit

Permalink
Correctly transmit spectator health
Browse files Browse the repository at this point in the history
  • Loading branch information
@DolceTriade
DolceTriade committed Dec 7, 2015
1 parent b467d3a commit 6a9b17b
Showing 1 changed file with 7 additions and 1 deletion.
8 changes: 7 additions & 1 deletion src/sgame/components/HealthComponent.cpp
View file
Expand Up @@ -18,7 +18,13 @@ void HealthComponent::HandlePrepareNetCode() {
gclient_t *client = entity.oldEnt->client;

if (client) {
if (client->ps.stats[STAT_HEALTH] != transmittedHealth) {
// if we are doing a chase cam or a remote view, grab the latest info
if (client->sess.spectatorState == SPECTATOR_FOLLOW) {
gentity_t *otherEnt = &g_entities[ entity.oldEnt->client->sess.spectatorClient ];
const HealthComponent *otherComp = otherEnt->entity->Get<HealthComponent>();
maxHealth = otherComp->maxHealth;
transmittedHealth = Math::Clamp((int)std::ceil(otherComp->health), -999, 999);
} else if (client->ps.stats[STAT_HEALTH] != transmittedHealth) {
client->pers.infoChangeTime = level.time;
}
client->ps.stats[STAT_HEALTH] = transmittedHealth;
Expand Down

5 comments on commit 6a9b17b

@Viech
Copy link
Member

@Viech Viech commented on 6a9b17b Dec 7, 2015

Choose a reason for hiding this comment

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

This looks like special-casing the spectator case within, potentially, every component used by a client. That is not acceptable. Spectator handling must be outside the regular component logic, if at all possible. I will need to disucss the best way to do this with @Kangz.

Please revert – not only should we avoid workarounds within CBSE whenever possible (we'll end up re-refactoring our own code instead of progressing with the conversion), but also you can't just change the component's maxHealth field within HandlePrepareNetcode, which is only ever supposed to export the component state to the network, not modify the component. I prefer to keep spectating broken and improve on the general client/spectator handling for next relase.

@DolceTriade
Copy link
Member Author

Choose a reason for hiding this comment

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

Definitely will revert when we have an appropriate alternative. Currently, spectator is handled by copying over the player who is being spectated's playerState_t. perhaps something analogous can be done with the entity?

@Viech
Copy link
Member

@Viech Viech commented on 6a9b17b Dec 8, 2015

Choose a reason for hiding this comment

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

If that's all that is necessary, we could add a SpectatorEntity with a SpectatorComponent that does this inside HandlePrepareNetCode.

Is there a lazy way to turn this thread into an issue? 😛

@DolceTriade
Copy link
Member Author

Choose a reason for hiding this comment

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

However, the issue with that is that HandleNetCode could be executed N times, where N is the number of times the player is being spectated. It would be annoying having to iterate, store specs somewhere else, and then iterate over the specs, which is further complicated by the fact that with stickySpec enabled, spectators can spectate spectators...

@Viech
Copy link
Member

@Viech Viech commented on 6a9b17b Dec 9, 2015

Choose a reason for hiding this comment

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

I think you misread my post. We certainly should not have multiple clients attached to any one entity or component. I was suggesting to add a SpectatorEntity used by spectating clients that would have a SpectatorComponent (and not much more) that would copy the networked state from the client/spectator being spectated to its own entity's (= the SpectatorEntity's) networked state.

Please sign in to comment.