Skip to content
This repository has been archived by the owner on Aug 16, 2021. It is now read-only.

chore: now AudioSource gets disabled when out of Scene Bounds #1420

Merged
merged 6 commits into from
Oct 15, 2020

Conversation

AjimenezDCL
Copy link
Contributor

@AjimenezDCL AjimenezDCL commented Oct 12, 2020

#1276

Now AudioSource is part of the scenery boundaries checks. If the entity has no mesh a simple check against the entity position is made.

The case for AudioStreaming/VideoStreaming is different since it's done in kernel and it will be tackled-down in #1419 Nevermind, it will be done here.

@github-actions
Copy link

@AjimenezDCL
Copy link
Contributor Author

Tests for DCLAudioStream won't be included here. The tests (along the refactor needed to make them doable) will be tackled down here #1421

Copy link
Contributor

@BrianAmadori BrianAmadori left a comment

Choose a reason for hiding this comment

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

Looks good!. Left some small comments.

@@ -202,6 +210,15 @@ protected virtual void UpdateEntityCollidersValidState(DecentralandEntity entity
}
}

protected virtual void UpdateComponents(DecentralandEntity entity, bool isInsideBoundaries)
{
IOutOfSceneBoundariesHandler[] audioSources = entity.gameObject.GetComponentsInChildren<IOutOfSceneBoundariesHandler>();
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
IOutOfSceneBoundariesHandler[] audioSources = entity.gameObject.GetComponentsInChildren<IOutOfSceneBoundariesHandler>();
IOutOfSceneBoundariesHandler[] components = entity.gameObject.GetComponentsInChildren<IOutOfSceneBoundariesHandler>();

Comment on lines 569 to 572
if (classId == CLASS_ID_COMPONENT.AUDIO_SOURCE || classId == CLASS_ID_COMPONENT.AUDIO_STREAM)
{
SceneController.i.boundariesChecker?.AddEntityToBeChecked(entity);
}
Copy link
Contributor

Choose a reason for hiding this comment

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

Not for this PR, but we should be configuring these aspect of components on some sort of manifest to avoid all the ifs like this. Take into account we already have this anti-pattern in messaging system -- the conditional voiding the lossy messages depending on message type.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I think we can use the same interface and simply check if the Monobehaviour retrieved (either by the componentFactory or the entity components list is IOutOfSceneBoundariesHandler.

I'm gonna give it a call in this PR and if it gets messy I will leave it and create a ticket.

}
else
{
Interface.WebInterface.SendAudioStreamEvent(model.url, true, 0);
Copy link
Contributor

Choose a reason for hiding this comment

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

This is supposed to stop the streaming? A comment or wrapper method would be useful here.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yeah, the last param is the volume, I will add a comment!

Comment on lines +9 to +12
public interface IOutOfSceneBoundariesHandler
{
void UpdateOutOfBoundariesState(bool enable);
}
Copy link
Contributor

Choose a reason for hiding this comment

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

❤️ 🏗️

@AjimenezDCL AjimenezDCL merged commit ed4ccdd into master Oct 15, 2020
@AjimenezDCL AjimenezDCL deleted the chore/1276_disable_outofscene_audiosource branch October 15, 2020 13:00
This was referenced Oct 16, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants