search

mika76 / mamesaver

Mamesaver is a mame emulated screensaver - get all the good ol' games playing their demo modes while you procrastinate and enjoy!
https://mika76.github.io/mamesaver/
MIT License
37 stars 10 forks source link

Game filtering #49

Closed nullpainter closed 5 years ago

nullpainter commented 5 years ago

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

Primary changes are:

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

mika76 commented 5 years ago

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 commented 5 years ago

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 commented 5 years ago

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

mika76 commented 5 years ago

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 commented 5 years ago

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 commented 5 years ago

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 commented 5 years ago

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

mika76 commented 5 years ago

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 commented 5 years ago

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 commented 5 years ago

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 commented 5 years ago

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 commented 5 years ago

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

nullpainter commented 5 years ago

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...

Yes, of course. I'll double-check the build settings.

Can you make the form re-sizable? Seems it's currently locked to one size.

Yes, I can do this.

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.

I may fix this instead! There should be nothing in the XML which is required - just additional metadata. I probably have some code somewhere which is assuming a value for, say, category.

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.

How many games do you have? I can't replicate this - with my ~260 games, the progress bar appeared immediately and immediately started updating.

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)

Yes, good point re: .msi. There is a command in a post build step to do the rename. I've never been able to open the installer project.

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.

There was a very good reason for this, but I'm going to have to take another look in order to explain it sensibly. 😄

nullpainter commented 5 years ago

  1. All build configurations are Any CPU, so I'm not sure why SysWOW64 is required. The only thing that's really changed is shifting from the troublesome ILMerge to ILRepack, so perhaps this is related? In any case, I may need to leave the installer/MSI work to you since I can't open it in my VS 2017.

  2. Config form is now resizable. It initially wasn't as I didn't see much point, but I can see that there may be unusually long categories or game titles where it may make sense. I've set a sensible minimum size.

  3. The point of the apply button is really just for the 'Selected games' option. If you have selected 'Selected games', click Apply and deselect a handful of games, you can click Apply again to reapply the selected games filter. If you have a more elegant UX suggestion, I'm all ears!

nullpainter commented 5 years ago

In this branch, I've just squashed two related bugs which caused unhandled failures (and no stack trace!) when any invocation of MAME hadn't terminated before the screensaver quit.