Skarsnik / QUsb2snes

A Qt based webserver for usb2snes. Users: go check usb2snes.com
https://skarsnik.github.io/QUsb2snes/
GNU General Public License v3.0
52 stars 34 forks source link

Configuration `RetroArchHosts` should take hostname #79

Closed bmarwell closed 3 years ago

bmarwell commented 3 years ago

Hi,

if I pass a host name to the config, retroarch will add an empty QHostAddress:

how to reproduce

[General]
retroarchdevice=true
RetroArchHosts="remoteName=docker"
debugLog=true

expected output

2021-04-12T07:38:24 RA Factory - Debug: Trying to connect to New RetroArch "remoteName" QHostAddress("host.docker.internal")

actual output

2021-04-12T07:38:24 RA Factory - Debug: Trying to connect to New RetroArch "remoteName" QHostAddress("")

problematic line

https://github.com/Skarsnik/QUsb2snes/blob/a4248c38d30745f0339c1cc854697cb6b95ba3bb/devices/retroarchfactory.cpp#L53-L59

Note: the assignment of a string to QHostAddress won’t work here as it expects an IP.

suggested solution

I am not a C++ programmer, BUT according to the docs something like this (pseudocode) should work:

QHostInfo info = QHostInfo::fromName(host.split('=').at(1));
QHostAddress address = info->addresses()[0];
newHost.addr = QHostAddress(address);

I think this should also work with IPs.

Skarsnik commented 3 years ago

The split('=') will cut your weird hostname and will still only contains remoteName

bmarwell commented 3 years ago

The split('=') will cut your weird hostname and will still only contains remoteName

I do not get it, what do you mean? Please annotate the corresponding line in the PR.

Skarsnik commented 3 years ago

RetroArchHosts="remoteName=docker" If I split on = I get a QStringList with RetroArchHosts | "remoteName | docker"

bmarwell commented 3 years ago

Then your existing code would also be erroneous. The variable "host" does not contain the full line, only remoteName=docker. Hence the split works as expected (and did in your version, too). See: https://github.com/bmarwell/QUsb2snes/commit/be4b01376d968e7071805d413489c305c0266e93#diff-78fee45624acc367fd0e8fc2e8fef07db1136cd6ccc57522719266db54f04a26L54-L56

bmarwell commented 3 years ago

BTW, you might want to add a unit test for this method :)