-
Notifications
You must be signed in to change notification settings - Fork 6
Total mod overhaul for split-screen support #14
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
base: master
Are you sure you want to change the base?
Conversation
|
@Xemrox I'll review this week, thank you for the significant contribution! |
mishahawthorn
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@Xemrox thank you again for the contribution! This PR introduces use of better conventions throughout and overall greatly improves the health of the project.
I did observe one potential regression which I'd be curious if you are able to reproduce, or clarify if the intention is to introduce different behavior here. When testing a local co-op scenario I found that opening the menu to pause would pause time for all users with this PR, where the 1.8.1 tag does behave as expected. (Mostly 😅 The "X" is not showing as expected, a bug in the current revision)
1.8.1 (current)
https://github.com/user-attachments/assets/600ffb39-a0d5-4100-94b1-b5080c6c3cdb
1.9.0 (proposed)
https://github.com/user-attachments/assets/b6f8656c-0b92-49bd-98c9-e99aea60fd2b
|
hey thanks for the feedback. I didn't notice the regression as I am using it solely on local co-op and didn't use the old version much. What would be the expected behavior? Maybe I've fixed a bug without knowing. Aside from that I've left a debug hotkey and a few verbose message changes in the mod which I like to remove first. |
|
Apologies for my delayed response, life got busy. Haven't had much time to play either 😔. Will take another look in the next week or so and see if we can get this merged! Thanks again @Xemrox! |
Seems like this bug is likely not a new one: https://www.nexusmods.com/stardewvalley/mods/21327/?tab=forum&topic_id=14176228 I'd love to be able to toggle pausing when any player has a pause-condition, rather than all players having it. :) I'll wait with trying to implement this until this PR is merged, as it seems I'll have to handle quite a few conflicts if not. |
I need to get out of the way here 😅 I have carved out time on my calendar to get another round of review this week! |
|
Just did some playtesting with one other farmhand over LAN multiplayer and found that the co-op regression now applies to that multiplayer mode now. Regardless of the farmhand's pause state, the shared game time would pause exclusively based on the host's paused state. I'll dig in and take a closer look at the code, and I haven't validated local co-op again yet. |
fixes all known split-screen bugs (#4) and releases as 1.9.0