Dinopony
commented
8 months ago Alright! Now that we do have a generic SNES connector for Bizhawk, I have ideas a bit clearer about how this should be added.
The library
I didn't look much into the library part yet (because I don't know much about SNI specifics), but you're saying:
The second project only holds the SNI gRPC code, and is mostly present due to a bug involving WPF and protobuf generation. I was also having some weird issues with the most recent version of grpc.net.client, so probably avoid updating that one.
That sentence is a bit too much in the specifics for me to understand what it means yet, but you're basically saying this lib could be avoided entirely if there wasn't a bug related with WPF. WPF being the UI framework we use, what's the connection (pun not intended) with SNI? How can the UI system mess up with protobuf generation (which appears to be some kind of serialization standard)?
The library is quite massive, and it feels a bit intimidating to include this directly inside the codebase. If it is absolutely needed, could it be turned into its own repo building to a nu.get package that we would use as a dependency?
As stated previously, I don't know much about SNI / Protobuf yet, so I'm mostly trying to understand what we can do.
The generic connector
The ISNIConnector is problematic for one reason: generic connectors don't usually embark / restrict the communication method being used, but rather the scope / platform it targets.
For instance, we now have a ISNESConnector which is capable of connecting to Bizhawk playing SNES ROMs on Snes9x core, and all SNES games will use that connector.
If ISNIConnector is made its own class, games will have to inherit either of them, making them incompatible with the other type.
This is why we prefer extracting the whole connection logic to helper classes which are then used inside connectors.
Here, what we'd want would be to make the ISNESConnector try to connect to SNI if no emulator is found, and hide all that complexity under the hood. This way, all games using this generic connector would be both compatible with the Windows RAM peeking solution and SNI (= RAM peeking on Linux + real hardware).
Having some kind of common interface with ProcessRamWatcher would be a plus and would completely hide the complexity for connector developers.
The game
I haven't yet taken a look at the game, but given the current situation, it could be made its own PR and added using the current ISNESConnector, which will become compatible with SNI down the road with subsequent PRs. This would reduce this PR's complexity (which would be nice, not gonna lie 😛 ) and make everyone's life easier.
Conclusion
Overall, it would be a very interesting addition to have SNI compatibility for HintMachine, and you seem to have done a technical base for that to happen, which is nice. Now we need to refine this a bit and insert this in an elegant and transparent manner for connectors' developers not to lose their hair too fast 😉
Silvris
Adds a connector to connect to alttpo/sni using the gRPC interface, and adds a game as an example.
The second project only holds the SNI gRPC code, and is mostly present due to a bug involving WPF and protobuf generation. I was also having some weird issues with the most recent version of grpc.net.client, so probably avoid updating that one.
Sanrio World Smash Ball! Quests Win Matches! (5). Score Goals! (10) Build Super Meter! (3200, 20 total meters). Collect Powerups! (10).
Balancing may be required but for now I think this is a pretty good spot for it.