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

Ability to pause Melody IC 1270 #1203

Open
Phoenix616 opened this issue May 23, 2020 · 6 comments
Open

Ability to pause Melody IC 1270 #1203

Phoenix616 opened this issue May 23, 2020 · 6 comments
Labels
status:pending Pending acceptance or closure. type:feature-request Request for something new

Comments

@Phoenix616
Copy link
Contributor

Is your feature request related to a problem? Please describe.
When wanting to pause a song played with the MIDI player IC 1270 it starts again at the start of the song rather than the position you were at.

Describe the solution you'd like
A way to start playing again at the position that you stopped at. This might be done via a parameter on the sign (PAUSE) but that means there would need to be a reset too, alternatively a redstone input from the side of the sign could pause it instead.

Describe alternatives you've considered
Using a different plugin or splitting up midi files (which wouldn't allow to arbitrarily pause)

Additional context
None?

@Phoenix616 Phoenix616 added status:pending Pending acceptance or closure. type:feature-request Request for something new labels May 23, 2020
@Phoenix616 Phoenix616 changed the title Ability to pause rather Melody IC 1270 Ability to pause Melody IC 1270 May 23, 2020
@me4502
Copy link
Member

me4502 commented May 24, 2020

The main concern here is that pausing it instead of resetting it requires keeping around a MIDI Sequencer, which has an open file handle and uses a decent amount of memory.

When they’re being used it’s generally fine, but when players can arbitrarily pause theoretically limitless amounts this could become a major issue in terms of performance. The open file handle problem could also cause some pretty major issues.

@Phoenix616
Copy link
Contributor Author

Maybe there could be a timeout or a max amount of concurrently paused ICs?

Also for my usecase this wouldn't really be a risk as on my server players don't have access to it, only builders/mods use it in custom minigames on my server so maybe a config option which defaults to disabled could help prevent this issue for the wider userbase.

@me4502
Copy link
Member

me4502 commented May 24, 2020

A timeout would be problematic, but having a max amount paused could technically work.

My concerns there would be around what would happen if that limit is hit, would it just reset them?

Also - this would require the IC being consistently loaded, which is going to add extra strain on the chunk system etc. I feel like adding this would overall create too many problems for it to be worthwhile

@Phoenix616
Copy link
Contributor Author

I would imagine the best behaviour would be resetting the oldest one when the limit is reached. As for the chunk: I don't really think keeping a couple chunks loaded until the limit is reached would be an issue if the limit isn't too big but I guess they could reset on unload?

Also for me personally these chunks would never unload while the IC is paused anyways. I realise that my usecase seems to be a bit fringe and might not represent how this could be used by a more normal player setup especially if it would put a strain on the system.

Also just to be clear: I'm open to actually implement a PR for this once we have a design which could work around the potential performance impact.

@me4502
Copy link
Member

me4502 commented May 24, 2020

I’d rather it not reset the oldest one when the limit is reached as that’s realistically more confusing behaviour, and also would require more tracking.

And sure, unloading the sequencer on IC unload sounds fine.

I’d have to check what the current sign syntax is to work out where there’d be space for it, but a PR would be appreciated.

@Phoenix616
Copy link
Contributor Author

I’d rather it not reset the oldest one when the limit is reached as that’s realistically more confusing behaviour, and also would require more tracking.

I think I meant "longest paused one" there, not oldest sequencer/longest playing one if you thought that, sorry.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
status:pending Pending acceptance or closure. type:feature-request Request for something new
Projects
None yet
Development

No branches or pull requests

2 participants