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

Do not add new music styles to old parks automatically #21043

Open
wants to merge 1 commit into
base: develop
Choose a base branch
from

Conversation

Gymnasiast
Copy link
Member

For some reason, newly introduced music styles were added to existing SV4 and SV6 files. This didn’t make a great deal of sense, as we always leave the object selection intact as much as possible. (E.g. we don’t add new land types or station styles or anything else either.)

While investigating, it also seems like SV4, SV6, TD4 and TD6 loading still assumes that the music IDs are fixed, which hasn’t been the case for a long time. This is something I’d want to investigate further and maybe address in a future PR.

@Gymnasiast Gymnasiast changed the title Do not add new music styles to old park automatically Do not add new music styles to old parks automatically Dec 3, 2023
@AaronVanGeffen
Copy link
Member

This appears to have been added intentionally: #19785

@karst
Copy link
Member

karst commented Dec 3, 2023

This was an intentional change, and it's currently also part of the release checklist for openmusic.

@karst
Copy link
Member

karst commented Dec 3, 2023

If you can somehow separate it from SV6/SV4 idm, but for the scenarios I definitely would like to keep it this way.

The reason we decided on enabling them by default is because it doesn't change anything balance wise about the game, and it used to not be an object type, but you were able to add two custom music tracks without cheating.

@Gymnasiast
Copy link
Member Author

Gymnasiast commented Dec 3, 2023

I understand the reasoning, but I still think it would be wise to reconsider. I think it should only be done for new scenarios. But if the general opinion of the players is that it should be kept, I’m not going to kick up a fuss. (Not that I should do so anyway.)

@IntelOrca
Copy link
Contributor

Adding to saved games was not intentional (otherwise it would have mentioned SV4/SV6 in the changelog).
Thus this PR only reverts the change, and does not fix the problem. Only scenarios should get the new styles.

@Gymnasiast
Copy link
Member Author

I meant that I think that the new music styles should only be selected when creating a new scenario, and loading from a SV4/SC4/SV6/SC6 should not. That is what my PR does, so from that perspective this PR does fix it. But judging from the reactions, I doubt many people would agree.

@IntelOrca
Copy link
Contributor

Oh okay, it is cleaner to not add the stuff...

But I guess the problem is that people kept asking how they add the new music. Not enough people seem to be aware or familiar with the object selection window and it is a bit of hack to do something fairly common. Do you have some alternative solutions?

@Gymnasiast
Copy link
Member Author

I think it would be a good idea to make it easier for the user to add new music styles to any game. E.g. have a button near the music dropdown to add more styles (which would then show the whole list of installed music, allowing the user to just pick a few extra ones). That would work for any style, not just official ones. I think that is a better solution overall, it gives the user more control over their saves while also making it easier for them to add new music, regardless of where it comes from.

@karst
Copy link
Member

karst commented Dec 3, 2023

I like that idea. Could be done after #20980 is merged.
Perhaps it could have a button that opens a locked down version of the object selector that only allows seeing the ride music tab? Or something similar, a sidetab that is a small version of the ride music object selector.

@ocalhoun6
Copy link
Contributor

ocalhoun6 commented Dec 5, 2023

Not enough people seem to be aware or familiar with the object selection window and it is a bit of hack to do something fairly common. Do you have some alternative solutions?

Might be nice to have a simplified/easy method for adding objects in general, not just for music... (Maybe a plugin could do this?)

Like in a few different windows:

  • ride selection
  • scenery selection
  • music selection (within a ride's window)
  • terrain selection

In each of those windows, somewhere in the UI, you could put a [+] or "Get more" button. And if you click that button, it opens a simplified version of the object selection window, showing only objects of that type and only allowing you to add more objects of that type, with all the other features of the object selection window hidden behind an 'advanced mode' button that would open up the full object selection window.

That would be easy to use and fairly intuitive -- anywhere you see a selection of objects to use in your park, you could have the [+] button somewhere next to it, which allows to to add more options to be used for that type of object.

@karst karst added the discussion Some input from team members is wanted. label Dec 13, 2023
oxzi added a commit to oxzi/nixpkgs that referenced this pull request Jan 13, 2024
- https://github.com/OpenRCT2/OpenRCT2/releases/tag/v0.4.6
- https://github.com/OpenRCT2/OpenRCT2/releases/tag/v0.4.7

Updating to 0.4.6 resulted some headaches, as suddenly lots of errors
about missing music objects were thrown and old maps were not loadable.
This was also mentioned in NixOS#263025.

Today I git bisect'ed OpenRCT2's source between 0.4.5 and 0.4.6, finding
5adbea9ac829b76507543e4c09b2000138ffbbcf as the culprit, which origins
in OpenRCT2/OpenRCT2#19785. From there I got to OpenRCT2/OpenRCT2#21043
which addresses this very issue and fixes it by undoing the original
change next to some later changes. This PR is now included as a patch
and, et voilà, OpenRCT2 works again!

Closes NixOS#263025.
@oxzi oxzi mentioned this pull request Jan 13, 2024
13 tasks
Copy link

This pull request has been marked as stale and will be closed in 14 days if no action is taken. To keep it open, leave a comment or remove the stale-pr label. If you're awaiting feedback from a developer, please send us a reminder (either here or on Discord).

Copy link

This pull request has been marked as stale and will be closed in 14 days if no action is taken. To keep it open, leave a comment or remove the stale-pr label. If you're awaiting feedback from a developer, please send us a reminder (either here or on Discord).

Copy link

This pull request has been marked as stale and will be closed in 14 days if no action is taken. To keep it open, leave a comment or remove the stale-pr label. If you're awaiting feedback from a developer, please send us a reminder (either here or on Discord).

@733737
Copy link
Contributor

733737 commented Apr 17, 2024

keep open

Copy link

This pull request has been marked as stale and will be closed in 14 days if no action is taken. To keep it open, leave a comment or remove the stale-pr label. If you're awaiting feedback from a developer, please send us a reminder (either here or on Discord).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
discussion Some input from team members is wanted.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

6 participants