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

Bring libtimidity support back #7636

Closed
rsn8887 opened this issue Jul 3, 2019 · 9 comments
Closed

Bring libtimidity support back #7636

rsn8887 opened this issue Jul 3, 2019 · 9 comments

Comments

@rsn8887
Copy link

@rsn8887 rsn8887 commented Jul 3, 2019

This is a reminder to bring libtimidity support back, to enable me to more easily merge the code of my Switch port into this main repo. The Switch port relies on libtimidity for music.

Not having libtimidity causes a plethora of merge errors during my tries to rebase.

See discussion here #7333, where it was concluded that a port is needed that uses libtimidity to warrant bringing it back. Well thenSwitch port uses it and I am trying to upstream it so there’s that.

And the original commit that removed libtimidity here. #7326. A simple revert doesn’t seem to work anymore:

@Milek7

This comment has been minimized.

Copy link
Contributor

@Milek7 Milek7 commented Jul 3, 2019

It is restored in emscripten PR: https://github.com/OpenTTD/OpenTTD/pull/7510/commits

@rsn8887

This comment has been minimized.

Copy link
Author

@rsn8887 rsn8887 commented Jul 3, 2019

Perfect I didn’t see it! Hmm so my rebase should work. Weird. Closing the issue.

@rsn8887 rsn8887 closed this Jul 3, 2019
@rsn8887 rsn8887 reopened this Jul 3, 2019
@rsn8887

This comment has been minimized.

Copy link
Author

@rsn8887 rsn8887 commented Jul 3, 2019

Ah I see it is in a PR that hasn’t been merged yet. Ok that explains it then. I will wait for the PR to be merged.

@Eddi-z

This comment has been minimized.

Copy link
Contributor

@Eddi-z Eddi-z commented Jul 3, 2019

i'm not quite sure how you come to the conclusion that someone else should do this for you, instead of you just reverting the commit that removed it?

@rsn8887

This comment has been minimized.

Copy link
Author

@rsn8887 rsn8887 commented Jul 4, 2019

I am calling for help here, because tried to revert it myself already, but so much has changed since it was first removed (especially in the configure script or config.libs or whatever it was called) that nothing compiled anymore after I tried to fix the conflicts.

Configure scripts are a constant pain when rebasing/reverting etc. because everything touches them.

@Milek7

This comment has been minimized.

Copy link
Contributor

@Milek7 Milek7 commented Jul 4, 2019

f04cda523 Revert aa350528: Remove: libtimidity support (NOT timidity support)
c324480c9 Fix: Use sound mixer in libtimidity
673347a7d Codechange: Convert virtual comments to override keywords in libtimidity.h
b00491221 Codechange: Replace NULL with nullptr in libtimidity

Cherry-picking those commits from my PR should apply cleanly on master.

@rsn8887

This comment has been minimized.

Copy link
Author

@rsn8887 rsn8887 commented Jul 4, 2019

Thanks, that is exactly what I was looking for :)

@TrueBrain

This comment has been minimized.

Copy link
Member

@TrueBrain TrueBrain commented Aug 27, 2019

I assume this means we can close this ticket now :) Tnx @Milek7 for helping @rsn8887 out!

@TrueBrain TrueBrain closed this Aug 27, 2019
@rsn8887

This comment has been minimized.

Copy link
Author

@rsn8887 rsn8887 commented Aug 28, 2019

Why was this closed? PR #7510 hasn’t been merged yet.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
4 participants
You can’t perform that action at this time.