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

Adds Handheld Radio/Listener system #1457

Merged
merged 9 commits into from
Jul 28, 2020
Merged

Adds Handheld Radio/Listener system #1457

merged 9 commits into from
Jul 28, 2020

Conversation

Bright0
Copy link
Contributor

@Bright0 Bright0 commented Jul 22, 2020

episode 2 bc gitkraken obliterated my older PR with nonsense

@RemieRichards
Copy link
Member

@Bright0 check out my suggestions above

@DrSmugleaf DrSmugleaf added the T: New Feature Type: New feature or content, or extending existing content label Jul 23, 2020
@Bright0
Copy link
Contributor Author

Bright0 commented Jul 23, 2020

Alright so I think things make a lot more sense now. ChatManager only checks if the listener is within the voice range and passes the distance to the listening component so that it can compare with its individual IListen interfaces. I didn't need a GetListenRange method on the listening component so I deleted it, the interface's GetListenRange method now works as intended since the listening component iterates through all of the attached relevant components and checks their individual listen ranges. @RemieRichards Everything should be good if you wanna check it out again

Copy link
Member

@ShadowCommander ShadowCommander left a comment

Choose a reason for hiding this comment

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

Looks pretty good. It's possible to simplify and use the existing EntitySystem EntityQuery instead of subscribing and unsubscribing. The same thing can probably done for the listener.

Content.Server/GameObjects/EntitySystems/RadioSystem.cs Outdated Show resolved Hide resolved
Content.Server/GameObjects/Components/RadioComponent.cs Outdated Show resolved Hide resolved
Content.Server/GameObjects/Components/RadioComponent.cs Outdated Show resolved Hide resolved
Content.Server/GameObjects/Components/RadioComponent.cs Outdated Show resolved Hide resolved
Content.Server/GameObjects/Components/RadioComponent.cs Outdated Show resolved Hide resolved
@RemieRichards
Copy link
Member

@Bright0 I personally wouldn't have removed the GetListenRange, as it would have allowed listeners to hear wider than voice range (which, while probably rare for balance reasons, could have been useful)

but it's fine I suppose. cross that bridge if we come to it.

@Bright0
Copy link
Contributor Author

Bright0 commented Jul 24, 2020

image
ready to merge unless there's anything else I should do

Content.Server/GameObjects/EntitySystems/RadioSystem.cs Outdated Show resolved Hide resolved

public override string Name => "Listening";

private ListeningSystem _listeningSystem = default!;
Copy link
Member

Choose a reason for hiding this comment

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

Remove _listeningSystem reference here and do what was done for RadioComponent to replace the subscribe unsubscribe.

{
base.Initialize();

_radioSystem = _entitySystemManager.GetEntitySystem<RadioSystem>();
Copy link
Member

Choose a reason for hiding this comment

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

Gonna need @PJB3005's feedback on this. Should the RadioSystem be saved like this or should it just be fetched every time on line 56.

Copy link
Member

@ShadowCommander ShadowCommander left a comment

Choose a reason for hiding this comment

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

If you do this, as I think Remie said before, it's possible to remove the ListeningComponent completely.

Comment on lines +33 to +34
listener.GetComponent<ListeningComponent>()
.PassSpeechData(message, source, dist);
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
listener.GetComponent<ListeningComponent>()
.PassSpeechData(message, source, dist);
foreach (var listener in entity.GetAllComponents<IListen>())
{
if (dist > listener.GetListenRange())
{
continue;
}
listener.HeardSpeech(message, source);
}

@Acruid Acruid merged commit 86a6ac4 into space-wizards:master Jul 28, 2020
rbertoche pushed a commit to rbertoche/space-station-14 that referenced this pull request Apr 10, 2023
TheArturZh referenced this pull request in TheArturZh/space-station-14 Oct 2, 2023
Morb0 pushed a commit to Morb0/space-station-14 that referenced this pull request Oct 24, 2023
rbertoche pushed a commit to rbertoche/space-station-14 that referenced this pull request Jun 24, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
T: New Feature Type: New feature or content, or extending existing content
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants