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

Game filtering #49

Open
wants to merge 39 commits into
base: master
from

Conversation

Projects
2 participants
@nullpainter
Contributor

nullpainter commented Oct 24, 2018

This PR is primarily to assist with game management for users without curated sets of ROMs.

Primary changes are:

  • Filters out mechanical games
  • Adds better checks for BIOS ROMs and runnable games
  • Rewrote config form to use WPF
  • Added per-column subtractive game filtering
  • Added global all/selected games filter
  • Added game category and subcategory to game list via bundled catver.ini
  • Added rotation to game list
  • Added number of selected games to label
  • Migrated off legacy packages.config to PackageReferences
  • Replaced troublesome ILMerge with ILRepack
  • Small fixes
  • Shuffled options to better make use of space

Due to the number of moving parts with game filtering, this should be considered a beta release and tested accordingly.

Edit: options shuffled, screenshots updated. I haven't updated the screenshots in the documentation as I'm not sure what your feeling is about the current tab contents - I've had to make everything quite a bit larger to accommodate the additional game list columns, but this is making all of the other tabs larger than is warranted for what's on them.

Edit: all filtering rewritten. There's more code in some of the code behind than I would like, which is resulting in unnecessarily complexity, compromises and confusing chat between the code behind and view models. This is largely to work around the DataGridExtensions package which I am using for the filtering. Since I have bypassed most of the library already, I will tidy this up shortly and reimplement what's missing.

Note that ILRepack integration has been shifted to a new Repack build target. This allows integration tests to work against debug and release builds. The version of the distributed screensaver must be built from the Repack target. This may require building twice - one for tests, and another for distribution.

image

nullpainter added some commits Sep 18, 2018

Fixed game list refresh and sort after rebuild, fixed global checkbox…
… behaviour after rebuild, fixed global checkbox state on start, changed game view model to be wrapper around game model
Removed unnecessary custom decade filter, replaced ILMerge with ILRep…
…ack, removed legacy config form, consolidated projects
Added filter view model, fixed viewmodel filter selection state, rein…
…stated secondary sorting, bypassed more of DataGridExtensions, added preliminary global filter

nullpainter added some commits Oct 25, 2018

Moved icon colour code and selection code out of code behind, fixed d…
…uplicate initialisation of VM and code behind
Removed DataGridExtensions and associate workarounds, shifted code fr…
…om code behind to view models, added tests, added separate ILRepack build configuration to allow tests to run against release build.
@mika76

This comment has been minimized.

Owner

mika76 commented Nov 22, 2018

Hey @nullpainter thanks for the update - I'll have a look as soon as I can. Do you think we should update the version number? Maybe 3.0 considering the UI rewrite?

@nullpainter

This comment has been minimized.

Contributor

nullpainter commented Nov 22, 2018

Maybe 3.0 considering the UI rewrite?

You asked this last time and I was reluctant, but there has been quite a few changes and the filtering does add a significant amount more flexibility (I'm biased but I'm loving it) so would be happy with 3.0 if you are 😄

@mika76

This comment has been minimized.

Owner

mika76 commented Nov 22, 2018

Yeah I think it's pretty warranted... 👍 💯

nullpainter added some commits Nov 22, 2018

@mika76

This comment has been minimized.

Owner

mika76 commented Nov 25, 2018

Hey @nullpainter do we need more than the .scr file now? I tried it but nothing happens. Doesn't run screensaver nor config form. Only got the scr though...

@nullpainter

This comment has been minimized.

Contributor

nullpainter commented Nov 25, 2018

Hi @mika76, how were you building it? I noted briefly in an update to this PR's description, but there's a new build target called Repack. This target creates the merged .scr file which is required for distribution.

This is because the integration tests don't work easily against an ILRepack'd build. If it's straightforward, you will need to change the build process to perform a release build, running tests (as currently), then a separate build using the Repack target. This will result in a bin\Repack directory, containing only the .exe and .scr.

If it's not straightforward to perform two builds, I can bodge the integration tests. Just requires copying all DI registrations from the main assembly.

@mika76

This comment has been minimized.

Owner

mika76 commented Nov 26, 2018

I was using the appveryor artifacts. I'll try and change the build and let you know, otherwise we should probably move over to a yml file...

@nullpainter

This comment has been minimized.

Contributor

nullpainter commented Nov 26, 2018

Not a bad idea if we want decentralised management of the build process.

@mika76 mika76 added this to In progress in v3 via automation Dec 11, 2018

@mika76 mika76 added this to the v2.1 milestone Dec 11, 2018

@mika76

This comment has been minimized.

Owner

mika76 commented Dec 11, 2018

Hey @nullpainter sorry I didn't test till now. After adding the repack configuration I got the proper exe and managed to get it working. Overall it works great and looks great.

Some comments:

  1. Can you make the form re-sizable? Seems it's currently locked to one size.
  2. Could it maybe do a rebuild list the first time you run it - this should then update the previous versions xml file. When I opened it I tried filtering a bit and it was buggy and then I clicked clear filter and the window closed. I realised it's probably because things are missing from the xml file, so after doing a Rebuild list all functions work fine.
  3. Rebuilding list kinda looked like it was frozen for a while until the progress bar suddenly updated after a while. Any way to show it's working while it's busy? I think the progress bar has some property for that.
  4. Currently the Rebuild config creates the msi, but it should probably be the repack one and I think the .exe needs to be renamed to .scr (this might be done by the installer itself - I forgot to check)
  5. I wonder if you need the apply button? All filtering and other actions work immediately when selecting, only the show all and show selected option required the apply.

Overall I think this is amazing - the filtering looks and works great.

@mika76

This comment has been minimized.

Owner

mika76 commented Dec 11, 2018

Just had a weird issue - mamesaver.scr config and run work fine when running off my desktop - but when I move the file to the System32 folder then trying to config or run causes some weird .net exception...

@nullpainter

This comment has been minimized.

Contributor

nullpainter commented Dec 11, 2018

Just had a weird issue - mamesaver.scr config and run work fine when running off my desktop - but when > I move the file to the System32 folder then trying to config or run causes some weird .net exception...

That's because it's currently being built as 64 bit only. Copying to SYSWOW64 works. I could change it to 32 bit.

@mika76

This comment has been minimized.

Owner

mika76 commented Dec 12, 2018

You mean 32 bit? (syswow64 is for 32 bit programs running on 64 bit OS) Does Any CPU not work in our case? I guess if not 32 bit is the best option since it should work on all machines then...

@mika76

This comment has been minimized.

Owner

mika76 commented Dec 12, 2018

Confirmed - works fine in syswow64 folder. I guess this is ok, as long as the installer knows where to put it.

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