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

Addition of the Jukebox #20418

Closed
wants to merge 14 commits into from
Closed

Addition of the Jukebox #20418

wants to merge 14 commits into from

Conversation

iNV3RT3D
Copy link
Contributor

@iNV3RT3D iNV3RT3D commented Sep 23, 2023

About the PR

Added the jukebox, a machine capable of playing any music added to its music list.

Why / Balance

After a suggestion, I tried my hand at creating a Jukebox for use in the bar. This would likely see more use when there is no musician in the game to fill the role. The Jukebox can only play files defined in the Standard.yml file, so additional songs have to be added there.

Being able to drag around a Jukebox playing music to all who would rather not listen would be quite annoying. So, the Jukebox has a power requirement to prevent such an issue.

Technical details

The Jukebox component makes use of a new prototype, the musicList. The musicList stores several MusicListDefinitions. MusicListDefinitions store a string display name, a string file path, a string for copyright, and a float for song length in seconds.

The Jukebox by default uses the Standard.yml file for its list of music, but additional .yml files could be created for separate and unique song lists. When a song is selected, a message storing the list ID of the song is sent to the server. Pausing and resuming a song is handled by storing the time the song was played. When instructed to play, the Jukebox will check and store the current time, subtracted by any existing offset, in the components SongStartTime variable. Then, when told to pause, it will compare the current time to the stored SongStartTime. This will get the offset for the next time it plays, stored in the SongTime variable. Setting the time via the slider updates the SongTime, recalculates SongStartTime based on the offset, and then restarts the audio stream at the new time.

The current list of songs has a size of 14.3 MB combined.

Media

  • [✓ ] I have added screenshots/videos to this PR showcasing its changes ingame, or this PR does not require an ingame showcase
2023-09-22.16-48-15-1.mp4

Breaking changes

None

Changelog

🆑

  • add: The Jukebox, a machine capable of playing a wide variety of songs, is now available.

@github-actions github-actions bot added Changes: Sprites Should be reviewed or fixed by people who are knowledgeable with spriting or visual design. Changes: UI Can be reviewed or fixed by people who are knowledgeable with UI design labels Sep 23, 2023
@Just-a-Unity-Dev
Copy link
Contributor

license of the music??

Copy link
Member

@AJCM-git AJCM-git left a comment

Choose a reason for hiding this comment

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

Nice, i have always wanted the jukebox, it gives a vibe to the bar y'know?, tho the implementation has some issues, but before that i got two questions for ya:

  1. Is this ported from some other fork? If so, do mention em
  2. Did you use AI to help you out in some places?

Resources/Prototypes/Catalog/Jukebox/Standard.yml Outdated Show resolved Hide resolved
Content.Shared/Jukebox/SharedJukeboxSystem.cs Show resolved Hide resolved
Comment on lines +4 to +8
using Robust.Shared.Timing;
namespace Content.Client.Jukebox;


public sealed class JukeboxSystem : SharedJukeboxSystem
Copy link
Member

Choose a reason for hiding this comment

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

.

using Robust.Shared.Timing;
using Range = Robust.Client.UserInterface.Controls.Range;

namespace Content.Client.Jukebox.UI
Copy link
Member

Choose a reason for hiding this comment

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

filescope your namespaces (this applies to the other files as well)

namespace Content.Client.Jukebox.UI
{
[GenerateTypedNameReferences]
public sealed partial class JukeboxMenu : DefaultWindow
Copy link
Member

Choose a reason for hiding this comment

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

make it a FancyWindow

{
SendMessage(new JukeboxComponent.JukeboxPlayingMessage());

//var playing = !jukeboxComp.Playing;
Copy link
Member

Choose a reason for hiding this comment

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

.

}

[DataDefinition]
public sealed partial class MusicListDefinition
Copy link
Member

@AJCM-git AJCM-git Sep 23, 2023

Choose a reason for hiding this comment

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

i dont think this is really necessary? name can perfectly be a field in the component, path needs to be of ResPath type, copyright is never used and cant you get the song lenght directly from the file or something?
Not only that, the prototype doenst seem necessary either, just use a SoundCollectionPrototype instead

@AJCM-git AJCM-git added the Status: Awaiting Changes This PR needs its reviews addressed or changes to be made in order to be merged. label Sep 23, 2023
@potato1234x
Copy link
Contributor

video no load

@VektorZ1
Copy link

Nice

@iNV3RT3D
Copy link
Contributor Author

Nice, i have always wanted the jukebox, it gives a vibe to the bar y'know?, tho the implementation has some issues, but before that i got two questions for ya:

  1. Is this ported from some other fork? If so, do mention em
  2. Did you use AI to help you out in some places?

This is not ported from any other fork.
I did not use AI. If anything seems AI like, it's just because I'm bad.

Working on the requested changes now.

@github-actions
Copy link
Contributor

github-actions bot commented Sep 23, 2023

RSI Diff Bot; head commit b2497b7 merging into b7cff81
This PR makes changes to 1 or more RSIs. Here is a summary of all changes:

Resources/Textures/Structures/Machines/jukebox.rsi

State Old New Status
off Added
on Added
select Added

Edit: diff updated after b2497b7

@github-actions github-actions bot added Status: Needs Review This PR requires new reviews before it can be merged. and removed Status: Awaiting Changes This PR needs its reviews addressed or changes to be made in order to be merged. labels Sep 24, 2023
@CaptainSqrBeard
Copy link
Contributor

Does it work fine when you walk away so you wouldn't hear a sound and then return back?

Copy link
Member

@EmoGarbage404 EmoGarbage404 left a comment

Choose a reason for hiding this comment

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

i'm pretty sure we already have every song you added to the jukebox. in any case, just use the lobby music and the space ambience instead of adding new tracks

@iNV3RT3D
Copy link
Contributor Author

i'm pretty sure we already have every song you added to the jukebox. in any case, just use the lobby music and the space ambience instead of adding new tracks

The lobby music and ambience are stereo audio files. Trying to play a stereo audio file as a location-based audio causes a crash.

@iNV3RT3D
Copy link
Contributor Author

Also, I learned fairly recently that there are going to be changes to how audio is handled. I'm going to wait for those changes to be implemented and then redo the jukebox to make use of that instead.

@iNV3RT3D iNV3RT3D closed this Sep 27, 2023
@Pspritechologist
Copy link
Contributor

Pspritechologist commented Oct 22, 2023

I implemented pretty much this exact thing ages ago for Parkstation, but it's unmerged :P
You can check there for any ideas if you'd like, and if you're a fan of @DEATHB4DEFEAT's UI they worked on.

My implementation includes cover art for the images, some additional functionality, EMAGged songs, signals for the Jukebox, upgradable parts (which is just funny :P), lots of public functions, and Timestamp markers instead of accumulators, that kinda stuff.
I imagine an ideal implementation would probably be a bit between the two :P

Simple-Station/Parkstation#208

@AJCM-git
Copy link
Member

@iNV3RT3D hey, the audio rework is finally done, telling you in case you are still interested in doing this

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Changes: Sprites Should be reviewed or fixed by people who are knowledgeable with spriting or visual design. Changes: UI Can be reviewed or fixed by people who are knowledgeable with UI design Status: Needs Review This PR requires new reviews before it can be merged.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

8 participants