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

Check Message LookTargets' Faction to Determine if it Should be Hidden #505

Closed

Conversation

SaberShip
Copy link
Contributor

When playing with multifaction, I have seen several messages that should not be visible to my faction.

A few examples are:

  • Pawns recovering from downed state
  • Mining uncovering new vein of material
  • Plants killed by cold
  • Prisoner resistance broke
  • Updates on pregnancy
  • Many many more

I found the patch responsible for filtering these and have reworked it to extract the message's "LookTargets" and determine if it should be displayed for the real player's faction based on that.

Dealing with GlobalTargetInfos and catching edge cases makes the code a little messy so I'm open to improvements!

@SokyranTheDragon
Copy link
Member

There's several issues with this.

First, by removing non-historical check:

  • You're making the messages desync prone. Archived messages that exist for one player will no longer exist for another, so if we do stuff with archive different players may get a different result since the state of their archive is different.
  • After re-hosting or rejoining all historical messages that clients had will be lost unless the host also had them, and they'll get all messages that host had even if they didn't.
  • If you change faction it'll keep historical messages from your previous faction, and won't give you archived messages from your new one.

Second, by removing the check if executing a command not issues by self it'll cause players to receive many messages irrelevant to them. With this change, for example, if I try to designate area and get a warning message, it'll display the warning for everyone. Not just me.

Third, your faction checks completely ignores the existence of non-player factions, meaning it'll completely silence messages if the map is factionless/is an NPC faction. Not all world objects can have a parent faction either, after all.

Fourth, you're assuming that those messages are never relevant to another player. Sure, the map may belong to another player, but what if someone else sends their pawns there and would get a message that would be relevant to them? What if 2 players decided to build a base on the same tile together, which is technically owned by one of them? If something deteriorated in storage then only the owner of the map would get notified, and the person whose things deteriorate won't be (unless it happened for the map owner).

@SaberShip
Copy link
Contributor Author

Thanks Soky!
It sounds like there are a lot more things I need to consider.

I'll close this PR for now.

@SaberShip SaberShip closed this Sep 3, 2024
@SokyranTheDragon
Copy link
Member

It was a good attempt, but would likely require additional work.

The messages would need to have a way to permanently associate a faction with them. Then we'd need to allow them to the archive, but hide them (and not display them when they're created) if they're aimed at the wrong faction. Or perhaps faction-unique archive?

As for the fourth point, that one would be more tricky and I don't really see a simple solution... We'd need to know what message it is first, but the only way to do it would be to check the text, which is a pretty bad solution overall...

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