Quote from "Blzut3"
But ensuring we can support partial master list updates comes to mind (in theory we can start querying servers as soon as the first master packet comes in).

Quote from "Blzut3"
I believe our server query protocol code doesn't really provide means for packet splitting either (since it's not really needed).

class MAIN_EXPORT GameRunner : public QObject
{
    public:
        GameRunner(const Server* server);

        virtual bool connectParameters(QStringList &args, PathFinder &pf,
            bool &iwadFound, const QString &connectPassword,
            const QString &wadTargetDirectory);

    protected:
        virtual QString argForConnect() const { return "-connect"; }
        virtual QString argForConnectPassword() const { return QString(); }
        virtual QString argForIwadLoading() const { return "-iwad"; }
        virtual QString argForPort() const { return "-port"; }
        virtual QString argForPwadLoading() const { return "-file"; }
        virtual QString argForDemoPlayback() const { return "-playdemo"; }
        virtual QString argForDemoRecord() const { return "-record"; }
}

class MAIN_EXPORT StringGetHandler
{
    public:
        virtual QString get() = 0;
};

class MAIN_EXPORT GameRunnerConnectParametersHandler
{
    public:
        virtual void run(QStringList &args, PathFinder &pf,
            bool &iwadFound, const QString &connectPassword,
            const QString &wadTargetDirectory) = 0;
};

class MAIN_EXPORT GameRunner : public QObject
{
    public:
        GameRunner(const Server* server);

        void connectParameters(QStringList &args, PathFinder &pf,
            bool &iwadFound, const QString &connectPassword,
            const QString &wadTargetDirectory)
        {
            d->connectParametersHandler.run(args, pf, iwadFound,
                connectPassword, wadTargetDirectory)
        }

    protected:
        void setArgForConnectHandler(StringGetHandler* handler)
        {
            d->argForConnectHandler = handler;
        }
        void setConnectParametersHandler(GameRunnerConnectParametersHandler* handler)
        {
            d->connectParametersHandler = handler;
        }
        // ... more handler setters
        QString argForConnect() const { // call handler here }
        QString argForConnectPassword() const { // call another handler here }
        // ... more handler callers
};

Quote
[16:29:57] <Zalewa> I've done some work today
[16:30:16] <Zalewa> with the simple stuff mostly out of the way I decided it's time to look into the way the games are started
[16:31:02] <Zalewa> the first thing that I think needed to be done was to separate game hosting from server connections
[16:31:30] <Zalewa> as a result some parts of GameRunner have now become GameHost
[16:31:43] <Zalewa> now, there are some things that need to be considered further
[16:32:39] <Zalewa> 1. There is method duplication for argument name retrieval for both classes. Some arguments, like "-iwad" or "-port" are needed in both cases. At first I was thinking that all arguments should be extracted to a completely separate class like GameArgs.
[16:34:13] <Zalewa> But then I thought that since some engines support separate executables for client and server then we shouldn't treat the arguments for connecting/hosting equally. I know it won't probably ever happen that argument for iwad or for port will be different between client and server executables, but then again we might get surprised one day.
[16:34:43] <Zalewa> Secondly, both classes still use pointer to Server*
[16:35:16] <Blzut3> hmm
[16:35:17] <Zalewa> I'm thinking about removing all hosting related stuff from Server* and create something new for this purpose, something that can be shared through value rather than through reference.
[16:36:50] <Blzut3> what problem are you trying to solve by having joining and hosting separate classes?
[16:38:42] <Zalewa> I'm only trying to reduce complexity. I figured it will be easier to extend either of these features when they're actually separate.
[16:40:19] <Zalewa> I'm also keeping in mind that we shouldn't have too many MAIN_EXPORT classes, so I'll probably stop before there's nothing more to split.
[16:44:55] <Zalewa> The thing is that the interfaces got pretty complicated even when you consider just hosting or just connecting. Just look at this: "bool connectParameters(QStringList &args, PathFinder &pf, bool &iwadFound, const QString &connectPassword, const QString &wadTargetDirectory);".
[16:45:30] <Blzut3> right
[16:47:24] <Blzut3> for removing the dependency on Server in hosting, I think we can improve how Binaries works a bit
[16:47:46] <Blzut3> in theory the Binaries object can be a member of the server itself, and then references can just be passed around
[16:48:00] <Blzut3> then the plugin itself can have a binaries instance for hosting
[16:48:31] <Blzut3> the only reason that binaries instance would need to be in the plugin is so that as much as possible can work by default
[16:48:53] <Zalewa> I think I'll be taking this connecting thing on next. Make what it does more obvious and make it extensible without breaking the ABI.
[16:49:48] <Zalewa> I see
[16:51:49] <Zalewa> I guess we could have a "Binary" class instead of "Binaries" (or some other name that better represents what it is) and then have instances/subclasses for each type.
[16:51:57] <Zalewa> offline, server, client
[16:53:17] <Blzut3> That actually sounds like a pretty good idea
[16:53:59] <Blzut3> with that testing binaries can be done by swaping out the instance rather than trying to have special support in there
[16:57:56] <Blzut3> probably the thing to do for the common command would be to store the argument names in the Binary class as well
[16:58:10] <Blzut3> since what to use depends more on the binary than the mode of operation
[17:00:37] <Zalewa> yes, that sounds good
[17:02:46] <Zalewa> okay, I think that covers current issues
[17:03:06] <Zalewa> going afk now, be back in about 30 minutes
[17:03:11] <Zalewa> maybe I'll think of something else
[17:03:17] <Blzut3> all right
[17:36:19] <Zalewa> back
[17:36:34] <Zalewa> I'm still considering what to do with the virtual methods.
[17:38:08] <Zalewa> Getting rid of them completely seems to be a too severe approach
[17:38:49] <Zalewa> I guess we could leave at least some of them as they are now, and use the method pointers whenever another virtual method needs to be added.

Quote from Blzut3
Odamex actually passes the hash for the password to the browser so that the password can be checked before actually joining the server.

int* x = new int;
QSharedPointer<int> p1(x);
{
    QSharedPointer<int> p2(x);
}
*p1 = 10;

GameClientRunner *Server::gameRunner()
{
    return new GameClientRunner(this);
}

QSharedPointer s(plugin->server(...));
s->self = s.toWeakRef();
return s;