vasl-developers / vasl

Virtual Advanced Squad Leader
http://vasl.info/
GNU Lesser General Public License v2.1
66 stars 28 forks source link

Revise functionality of BoardPicker dropdown list #1836

Closed derimmer closed 9 hours ago

derimmer commented 1 month ago

Revise sort code in ASLBoardPicker.refreshPossibleBoards. Two issues:

bd100+

a/b boards in the wrong place

derimmer commented 4 weeks ago

Third issue : key stroke "10" selects 10, 10a and 10b.

derimmer commented 3 weeks ago

https://www.gamesquad.com/forums/index.php?threads/board-organization.200191/

Dropdown functionality when using keystrokes rather than the mouse.

derimmer commented 3 weeks ago

From @uckelman "A major contributing factor to the board selection combo box being unresponsive is that BoardVersionChecker.updateBoard() is being run on the EDT, so blocks the UI. Better would be to run that as a background task which then posts an update task to the EDT on completion."

If only I knew how to do that . . . . well, I will find out. More learning coming up. All good. Thanks Joel.

uckelman commented 3 weeks ago

SwingWorker is potentially what you want for running background tasks that report back to the EDT when finished.

geezer09 commented 2 weeks ago

Created branch feature/issue_1836 for this issue and started working on it. I agreed with some comments on the forum and decided to delay the execution of the actionPerformed() method when a new selection is made in the combo box.

I think the UI is a bit goofy and am not super satisfied with this solution but it makes it alot better when scrolling through the list using the arrows. I think something to consider would be to give the user the option of downloading all boards, what is that 100-200MB? Another option to consider is to only download the board when the user double-clicks on a selection, i think that would make more sense in a way.

I also changed the ordering of the maps as they are displayed in the dropdown list. Basically I order by map type first (i.e. Geoboard, HASL, Deluxe, Starter Kit) and then alphabetically within those groups. This still needs a bit more work but is already better then what it was.

derimmer commented 2 weeks ago

As I am sure that you have already discovered, the source data use by Boardpicker to implement any kind of ordering of the boards in the list comes from the v5boardVersions.xml file in the boards directory in VASL. Feel free to play with this file as well, perhaps adding, deleting or amending the data elements.

If we are re-doing this functionality, now might be time to delete some of the old TPP boards that I suspect are not getting much use.

You asked about an all in one option. Personally, I would be against it. It would mean that every time a board is added or updated, that all-in-one file would have to be changed. I don't want to do that work and I don't suspect anyone else does. Perhaps you can automate that part of the process?

As for only downloading when the user double-clicks, that is adding another action that the user has to perform. At present there is no need for a user to double-click: they click on a board and it appears as a thumbnail, meaning it has downloaded if necessary.

I stress that the above are just my thoughts and I suspect others would disagree so please proceed as you prefer. You are the one doing the work and I have no desire to impose on that.

zgrose commented 2 weeks ago

You already probably seen my comment on GS, but it sounds a lot that people might like a search box and a list box instead of a combo box.

Or at least a search feature for those that have gotten used to the combo box.

geezer09 commented 2 weeks ago

I just pushed code that replaces the combobox with a searchbox and list.

derimmer commented 1 week ago

i pulled down the 1836 changes today.

First, of all, please feel free to pull these and other changes into develop. I think you have the hang of this.

As for these particular changes, I noticed two things.

  1. When trying to add new rows and columns, the map was not displaying correctly. It would not showing a board selection option for each board, nor was it displaying all of the selected boards (on second test I discovered that it was showing everything I selected but not where I thought they would be). Plus, it was resizing such that the initial board selection list was barely visible.

  2. If, during map selection I select Prev and then Select New Game, I still see my previous selections. Perhaps it actually works this way currently, I can't recall have done that before.

Plus, we have another issue on our issues list related to Boardpicker and that is to remove the Use Deluxe Size Hexes option (1794). With the advent of boardZoomer this is (a) no longer necessary and (b) likely to cause unintended results. If you want to go ahead and remove it as part of this work, that would be appreciated.

Once pulled into develop it will be in the subsequent beta.

geezer09 commented 1 week ago

So I made and pushed some changes so that the searchbox is now visible in each ASLboardSlot which should fix the issue you mentioned when having multiple maps to select. I also removed the deluxe hex size option. After you test it out again I can create a pull request if your happy with it.

A quick check confirmed that selecting previous and then new game returns the previous selections so I don’t this should be changed at this point.

On another note, what is the normal workflow for incorporating changes into the main branch. It seems like master or main was given up years ago and that develop is being used as master?? I’m fine with continuing whatever was done in the past but my preference is for short-lived “issues/bugs/new functionality” branches that get merged into develop with a pull request. This way there is always at least two set of eyes that sees code being pushed to the main branch and everyone can test out and comment on stuff people are working on. But again, happy to do whatever is normally getting done.

derimmer commented 1 week ago

So I made and pushed some changes so that the searchbox is now visible in each ASLboardSlot which should fix the issue you mentioned when having multiple maps to select. I also removed the deluxe hex size option. After you test it out again I can create a pull request if your happy with it.

I am happy to test it and let you know what I think but you are welcome to create pull requests whenever you wish. They will all get included in a beta build and users will give us feedback. But there is no internal control. You do the work the way you want it.

A quick check confirmed that selecting previous and then new game returns the previous selections so I don’t this should be changed at this point.

OK. Thanks for checking.

On another note, what is the normal workflow for incorporating changes into the main branch. It seems like master or main was given up years ago and that develop is being used as master??

The normal workflow is to create a branch from develop. Push your remote changes to the branch. Pull from the branch into develop. You are correct: we are using develop as master.
I’m fine with continuing whatever was done in the past but my preference is for short-lived “issues/bugs/new functionality” branches that get merged into develop with a pull request. This way there is always at least two set of eyes that sees code being pushed to the main branch and everyone can test out and comment on stuff people are working on. But again, happy to do whatever is normally getting done.

As I wrote elsewhere, you are welcome to use short term "issue" branches or a longer-term "version" branch. Depends on how much work you find it to create new branch after new branch. There are not so many people pushing so many changes that either approach is likely to cause confusion.

To be honest, I will have to be more careful about pulling changes made to develop down into my remote "version" branch than I have been in the past few years when I have done about 90% percent of the pushing/pulling. I may switch back to "issue" branches.

Please don't wait for people to test/review your branch changes. Sometimes they will but most often not, unless you specifically ask for us to do so.