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.

Sign up for GitHub

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

Jump to bottom

12544 - Hot-seat support - ability to specify default side #12646

Closed
wants to merge 89 commits into from
Closed

12544 - Hot-seat support - ability to specify default side #12646

wants to merge 89 commits into from

Conversation

riverwanderer
Copy link
Collaborator

@riverwanderer riverwanderer commented Sep 4, 2023 •
edited
Loading

First effort at providing a hotseat over-ride, addressing issue #12544.

If property VassalNextSide is defined, that will be used.

@riverwanderer
Provides facility to set the hot-seat default side via a module Globa…
91ef123 
…l Property VassalNextSide. The GP may be defined as a Preferences string property (on a blank tab to keep it hidden), in which case the VassalNextSide will take effect a per-player level.
@riverwanderer
Provides facility to set the hot-seat default side via a module Globa…
fc94dfb 
…l Property VassalNextSide. The GP may be defined as a Preferences string property (on a blank tab to keep it hidden), in which case the VassalNextSide will take effect a per-player level.
@riverwanderer
Provides facility to set the hot-seat default side via a module Globa…
026178c 
…l Property VassalNextSide. The GP may be defined as a Preferences string property (on a blank tab to keep it hidden), in which case the VassalNextSide will take effect a per-player level.
@riverwanderer
Provides facility to set the hot-seat default side via a module Globa…
b03eef8 
…l Property VassalNextSide. The GP may be defined as a Preferences string property (on a blank tab to keep it hidden), in which case the VassalNextSide will take effect a per-player level.
@riverwanderer
Validate VassalNextSide - check that it is an available side, rather …
07e3128 
…than just not unavailable.
@riverwanderer
Validate VassalNextSide - check that it is an available side, rather …
007630c 
…than just not unavailable.
@riverwanderer
Validate VassalNextSide - check that it is an available side, rather …
c5dcadb 
…than just not unavailable.
@riverwanderer
Copy link
Collaborator Author

riverwanderer commented Sep 4, 2023 •
edited
Loading

Bug to fix (occurs when VassalNextSide is not defined):

023-09-04 17:29:31,915 [14777-AWT-EventQueue-0] ERROR VASSAL.tools.ErrorDialog - 
java.lang.NullPointerException: Cannot invoke "String.isEmpty()" because "nextChoice" is null
	at VASSAL.build.module.PlayerRoster.promptForSide(PlayerRoster.java:780)

Also, on second thoughts, it will not be sensible to set this property locally, as then any given module that uses the feature will not be able to provide a hotseat function that works for both solo and multiplayer modes.

@riverwanderer
Copy link
Collaborator Author

Retested successfully with standard hotseat side-switching and with the VassalNextSide over-ride feature.
Vassal Reference Manual update to follow.

@riverwanderer riverwanderer requested review from Cattlesquat and removed request for Cattlesquat September 4, 2023 21:51
@riverwanderer riverwanderer added Ready for Review Ready to be reviewed for Merging enhancement New feature or request labels Sep 4, 2023
@riverwanderer riverwanderer changed the title 12544 next side choice 12544 Next side choice Sep 4, 2023
@riverwanderer riverwanderer changed the title 12544 Next side choice Hot-seat support - ability to specifiy default side Sep 4, 2023
@riverwanderer riverwanderer changed the title Hot-seat support - ability to specifiy default side 12544. - Hot-seat support - ability to specifiy default side Sep 4, 2023
@riverwanderer riverwanderer changed the title 12544. - Hot-seat support - ability to specifiy default side 12544 - Hot-seat support - ability to specifiy default side Sep 4, 2023
@riverwanderer riverwanderer added this to the 3.7.0 milestone Sep 4, 2023
@riverwanderer riverwanderer changed the title 12544 - Hot-seat support - ability to specifiy default side 12544 - Hot-seat support - ability to specify default side Sep 6, 2023
@uckelman uckelman modified the milestones: 3.7.0, 3.8.0 Sep 7, 2023
@riverwanderer riverwanderer removed the Review-In Progress Review is in progress label Sep 14, 2023
@riverwanderer
Copy link
Collaborator Author

This PR doesn't create any v3.7 conflicts, can it be re-classified for the 3.7.1 milestone?

@riverwanderer
Merge branch 'master' of https://github.com/vassalengine/vassal into …
0cbee71 
…12542-side-switch-race-mitigation

# Conflicts:
#	vassal-app/src/main/java/VASSAL/build/module/PlayerRoster.java
@riverwanderer
Re-aligned to master.
a561316
@riverwanderer
Re-aligned to master.
9604c83
@riverwanderer
Attempt block button is no sides and to prevent null playerSide in game.
12fe724
@riverwanderer
Attempt block button is no sides and to prevent null playerSide in game.
e80631d
@riverwanderer
Attempt block button is no sides and to prevent null playerSide in game.
070cfd2
@riverwanderer
Merge branch '12542-side-switch-race-mitigation' of https://github.co…
ba88b09 
…m/riverwanderer/vassal into 12542-side-switch-race-mitigation
@riverwanderer
Attempt block button if no sides and to prevent null playerSide in game.
0f47f77
@riverwanderer
Restore always do setLaunchBtton() in PlayerRoster(). Wrong place to …
a928de9 
…try not including the change sides button.
@riverwanderer
null playerside check correction.
4f315af
@riverwanderer
null playerside check correction. Observer needs to be filtered from …
dafa3c2 
…Wizard.
@riverwanderer riverwanderer added Conflicts Has Conflicts with master that need to be resolved and removed Ready for Review Ready to be reviewed for Merging Review-Changes Requested Reviewer has request changes labels Sep 20, 2023
@riverwanderer
Copy link
Collaborator Author

Merged and re-tested OK. Reference Manual update included.

Note that this change should have no effect on modules unless the designer opts-in by defining the controlling global property VassalNextSide.

@riverwanderer riverwanderer added Ready for Review Ready to be reviewed for Merging and removed Conflicts Has Conflicts with master that need to be resolved labels Sep 20, 2023
@riverwanderer riverwanderer modified the milestones: 3.8.0, 3.7.1 Sep 20, 2023
@riverwanderer riverwanderer self-assigned this Sep 20, 2023
@uckelman uckelman modified the milestones: 3.7.1, 3.7.2 Sep 20, 2023
@riverwanderer
Copy link
Collaborator Author

PR superseded by #12732.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@Cattlesquat Cattlesquat Awaiting requested review from Cattlesquat

Assignees

@riverwanderer riverwanderer

Labels
enhancement New feature or request Ready for Review Ready to be reviewed for Merging
Projects
None yet
Milestone
3.7.2
Development

Successfully merging this pull request may close these issues.

Allow module control over Side Switcher "nextChoice"
4 participants
@riverwanderer @Cattlesquat @uckelman @BrentEaston