Trunk Refactoring

[daniel.santos]

Does anybody mind if I go ahead and get rid of the "Glest" namespace in the trunk?  I suppose I can do it in all branches if it will make you guys feel better about merges.  This affects every single .h and .cpp file in the game and map_editor projects (I'm not talking about moving the glest_game and glest_map_editor directories as well, but if you guys want to check in your code, I can do that on all branches too so that future merges will be easier.)

Also, I'm moving over a lot of refactoring changes from the network branch that are not net-work related, mostly in shared_lib.

[silnarm]

Does anybody mind if I go ahead and get rid of the "Glest" namespace in the trunk?  I suppose I can do it in all branches if it will make you guys feel better about merges.  This affects every single .h and .cpp file in the game and map_editor projects (I'm not talking about moving the glest_game and glest_map_editor directories as well, but if you guys want to check in your code, I can do that on all branches too so that future merges will be easier.)

Also, I'm moving over a lot of refactoring changes from the network branch that are not net-work related, mostly in shared_lib.

Well, I'm happy for you to go ahead and do this, I'm also happy to bring the pathfinder and lua_ext branches into line when you're done, the refactor branch I hear-by declare dead, I've been looking at commands a lot again recently... and I've come to the conclusion that reformulating them as decision trees or state machines might be the best way forward... but more on that later, when I'm more sure of what I want to do with them...

- Happy hacking.

[daniel.santos]

Ahh, thanks!  I was really getting worried about the refactor branch to be honest.  I knew you had come up with a lot of good ideas on ways to do unit management and I haven't touched that in anything I've done, mostly with your changes in mind.  None the less, I've touched a lot of things around it so I was getting increasingly worried.  I'll be interested to hear your new ideas, but I also have a lot on my plate at the moment, so we can discuss that later.

So I know that I tend to be a bit random; I get an idea and I start working on it.  Silnarm, I think you tend to be that way too.  We both do really good work and come up with really good stuff.  But I'm going to try to pay closer attention to Hailstone's direction -- I think he's really good at this "coordination" stuff.  Maybe we should make serious plans for each release and have some mechanism for tracking, discussing, debating and deciding upon what will happen in each release -- one that gives you (Silnarm) and I enough room to run off on our crazy tangents, but gives Hailstone the ability to keep in check what actually goes into a release.

Also, sorry I got behind a bit on the forums.

[silnarm]

So I know that I tend to be a bit random; I get an idea and I start working on it.  Silnarm, I think you tend to be that way too.  We both do really good work and come up with really good stuff.  But I'm going to try to pay closer attention to Hailstone's direction -- I think he's really good at this "coordination" stuff.  Maybe we should make serious plans for each release and have some mechanism for tracking, discussing, debating and deciding upon what will happen in each release -- one that gives you (Silnarm) and I enough room to run off on our crazy tangents, but gives Hailstone the ability to keep in check what actually goes into a release.

Yeah, guilty as charged.  Direction is good, and I agree, young Hailstone does seem to be the most 'sensible' of us
I think Bugzilla is insufficient for our needs, Hailstone and I started discussing trackers on IRC on tuesday, but I got sidetracked (pun not intended, noticed reviewing post )... I've got a bunch of stuff in mind deserving of a 'ticket' but not really 'bugs' or 'features'.

Regarding targets, I can't check what is on bugzilla atm , but yeah, we should definitely be discussing some goals, and revising targets for releases... I'm not to sure how to proceed exactly at this point, I am leaning toward a total focus on creating an RTS Engine, those command updates as decision trees &| FSMs might well be Lua (C++ assisted) 'classes' &| functions.

While I'm not willing to back my claim up now, I think many of the things people have been requesting of late that require new code, require new/modified code for commands (&| skills). Command updates aren't frequent enough to concern me much on the performance side, so I think defining & updating commands in Lua is probably a good idea... but obviously I would like to talk with you and Hailstone about it.

Obviously that's no small task either, and should not really be done while other refactoring is being done, so it should probably go on the back-burner for now in any case.

[daniel.santos]

very good point about the commands/skills. So lets add that as an item then: lua-defined skill types and command types.

[charlieg]

Use Trac for ticket/bug tracking.  It's a hosted app on Sourceforge so little (or none) admin hassle there either.

[silnarm]

Use Trac for ticket/bug tracking.  It's a hosted app on Sourceforge so little (or none) admin hassle there either.

Thanks for the suggestion Charlie, I was actually leaning toward trac myself, Hailstone has found a Bugzilla plugin that we might take a look at, but I've enabled trac on sourceforge and had a bit of a play with it.  I really like the 'all encompassing' nature of it, tracker/task manager/wiki/etc/etc. I think it will be getting my vote

[hailstone]

Don't forget to have a look at mantis and you can try redmine with a bitnami stack http://bitnami.org/stack/redmine

Track doesn't seem to be able to do ticket searches based on time:
ie search for closed bugs in a certain month.

[silnarm]

Don't forget to have a look at mantis and you can try redmine with a bitnami stack http://bitnami.org/stack/redmine

Track doesn't seem to be able to do ticket searches based on time:
ie search for closed bugs in a certain month.

I'll enable mantis as well, and have a play/look. Trac looks pretty sweet but if such a simple query is not possible, maybe not. At the end of the day, whatever meets our needs is fine by me, while Trac might have my vote for now on the issue, as Project Coordinator, I'm hoping you'll been the one making the most use of it, so I say I get one vote, Daniel gets one vote, and you get three

Edit:
...Mantis... seems like saying it's half the project Trac is would be stretching things.... BitNami::Redmine on the other hand looks very shiny, but the glare is giving me a head ache... will have to get back to you.

[daniel.santos]

shoot, not enough time to catch up on the boards right now!

But I just wanted to post my status on this thread so everybody knows:

• I haven't committed any of these refactoring changes into the trunk.  Instead, I've created a new branch called tmp_program_refactor where I have committed them and it is currently broken with some 600-odd compile errors
• When I'm done fixing that, I'm going to have Silnarm & Hailstone review and if they approve, merge it back into the trunk as well as the network branch.  If they scream at me, I'll only merge it into the network branch
• At that point, I will resume the rest of the network stuff, hopefully with a nice shiny and new foundation to work from that will hopefully allow us to keep the network code more separated from stuff it shouldn't be coupled to (like sound, and graphics code), eventually allowing us to have a dedicated server and maybe even a dedicated master-server type of thing.

But I don't trust that my IP address wont change again leaving me unable to access my machine from my gf's house, so I'm committing my current state now, even though it's still broken.

[silnarm]

mwuh-haha... I just sneaked a merge of lua_ext into trunk 

I had to do it manually... merging has become an absolute nightmare... between sourceforge's laggy servers and the massive changes in the source tree...

I'm going to bring the bugfixes from 0.2.12 over to trunk too, and to 0.2.13... I don't envy you in the task you face to get trunk back together... I hope you get sourceforge on a good day

[charlieg]

Track doesn't seem to be able to do ticket searches based on time:
ie search for closed bugs in a certain month.

You can do a fair bit using the custom queries (View Tickets -> Custom Query) but possibly not this.

It seems like a very strange reason to turn down a ticketing tool though.  I just can't see that kind of measure being of any use to the project?  Other than morbid curiousity, why would it matter how many tickets get done in a certain month?

Since you can easily view tickets per version (a far more useful metric for a volunteer, spare-time project with no deadlines) I would suggest it contains all the reporting features you practically need.

[silnarm]

Tracs reporting facilities are quite good I think, we've been trying it out, and I really like it.

I think Hailstone wanting the tickets closed in the last month has a perfectly practical use, in assisting him prepare the 'Glest Community Development Monthly'.

This can be done anyway, TracQueries I think can not, but you can actually write your own SQL queries for reports, so there are no limits when it comes to reporting...

[silnarm]

This can be done anyway, TracQueries I think can not, but you can actually write your own SQL queries for reports, so there are no limits when it comes to reporting...

I wont mention how long I spent playing with this... eventually I figured out the Trac inserted dates in the database were actually UNIX timestamps...

Report {9}
Closed tickets modified in the last 30 days, it doesn't store a datetime for when the status changed to closed, this should be good enough... it wont miss any, you'll just get 'false positives' in the form of old bugs they may have had their ticket modified for some reason.

You can 'edit report' if you want to change the time period, if the SQL scares you too much, let me know

[daniel.santos]

Hey, I want to talk about my screwing with the trunk, not Trac!!   OK, so I need to play with it and check it out, I admit it.

Anyway, I'm down to 168 errors, w00t!  I've done a few bandaids here & there, but it's mostly a cleaner (and slightly leaner) design.  But here's basically where it's at with regards to changes

from source/game/main/program.h

Code: [Select]

class Program : private Uncopyable {
public:
    enum LaunchType {
        START_OPTION_NORMAL,
        START_OPTION_SERVER,
        START_OPTION_CLIENT,
        START_OPTION_DEDICATED
    };

private:
    Config config;
    Console console;
    Lang lang;
    Program::LaunchType launchType;

    static Program *singleton;    /**< The one and only Program object. */

protected:
    Program(Program::LaunchType launchType);

public:
    virtual ~Program();

    static Program &getInstance()       {assert(singleton); return *singleton;}

    Config &getConfig()                 {return config;}
    const Config &getConfig() const     {return config;}
    const Lang &getLang() const         {return lang;}
    Console &getConsole()               {return console;}
    const Console &getConsole() const   {return console;}
    bool isDedicated() const            {return launchType == START_OPTION_DEDICATED;}

    virtual int main() = 0;

protected:
    Lang &getNonConstLang()             {return lang;}
};

from source/game/main/gui_program.h (some irrelevant stuff snipped)

Code: [Select]

class GuiProgram : public Program, public WindowGl {
private:
    Keymap keymap;                          /**< The keymap that maps key presses to commands */
    FactoryRepository factoryRepository;
    GraphicsFactory &graphicsFactory;
    SoundFactory &soundFactory;
    Metrics metrics;
    Renderer renderer;
    SoundRenderer soundRenderer;
    CoreData coreData;

public:
    GuiProgram(Program::LaunchType launchType, const string &serverAddress);
    ~GuiProgram();

    static GuiProgram &getInstance() {
        Program &base = Program::getInstance();
        assert(!base.isDedicated());
        return static_cast<GuiProgram &>(base);
    }

    // getters
    Keymap &getKeymap()                     {return keymap;}
    GraphicsFactory &getGraphicsFactory()   {return graphicsFactory;}
    SoundFactory &getSoundFactory()         {return soundFactory;}
    const Metrics &getMetrics() const       {return metrics;}
    Renderer &getRenderer()                 {return renderer;}
    SoundRenderer &getSoundRenderer()       {return soundRenderer;}
    CoreData &getCoreData()                 {return coreData;}
    const Lang &getLang() const             {return getLang();}

    // misc
    int main();

    // more junk snipped...
};

So I modified FactoryRepository some, so it essentially creates the graphics & sound factories when it's created -- this remains private to GuiProgram.  Then, in the GuiProgram constructor, we get references to the graphics & sound factories and those values are relegated to whomever needs them -- those objects should pretty much be the data members of GuiProgram only.  The classes GraphicsInterface and SoundInterface have been ripped out (that's now managed by GuiProgram) and all of the crap it used to do to create factories & such is all managed privately in the GuiProgram constructor instead of being scattered all over the code and requiring 20 different classes to each expose a singleton value.  This all makes the GuiProgram constructor pretty huge, but that frankly doesn't bother me.

Code: [Select]

GuiProgram::GuiProgram(Program::LaunchType launchType, const string &ipAddress)
        : Program(launchType)
        , WindowGl(
            getConfig().getDisplayWindowed() ? wsWindowedFixed : wsFullscreen,
            0,
            0,
            getConfig().getDisplayWidth(),
            getConfig().getDisplayHeight(),
            getConfig().getDisplayRefreshFrequency(),
            getConfig().getRenderColorBits(),
            getConfig().getRenderDepthBits(),
            getConfig().getRenderStencilBits(),
            "Glest Advanced Engine")
        , programState(NULL)
        , preCrashState(NULL)
        , keymap(getInput(), "keymap.ini")
        , factoryRepository(
            getContext(),
            getConfig().getRenderGraphicsFactory(),
            getConfig().getSoundFactory())
        , graphicsFactory(factoryRepository.getGraphicsFactory())
        , soundFactory(factoryRepository.getSoundFactory())
        , metrics()
        , renderer(graphicsFactory)
        , soundRenderer(getConfig(), soundFactory)
        , coreData(getConfig(), renderer)
        , renderTimer(getConfig().getRenderFpsMax(), 1)
        , tickTimer(1.f, maxTimes, -1)
        , updateTimer(static_cast<float>(getConfig().getGsWorldUpdateFps()), maxTimes, 2)
        , updateCameraTimer(static_cast<float>(GameConstants::cameraFps), maxTimes, 10) {
    renderer.init();

    keymap.save("keymap.ini");

    // startup and immediately host a game
    if (launchType == Program::START_OPTION_SERVER) {
        MainMenu* mainMenu = new MainMenu(*this);
        setState(mainMenu);
        mainMenu->setState(new MenuStateNewGame(*this, mainMenu, true));

        // startup and immediately connect to server
    } else if (launchType == Program::START_OPTION_CLIENT) {
        MainMenu* mainMenu = new MainMenu(*this);
        setState(mainMenu);
        mainMenu->setState(new MenuStateJoinGame(*this, mainMenu, true, IpAddress(ipAddress)));

        // normal startup
    } else {
        setState(new Intro(*this));
    }
}

Anyway, I hope to get this all patched up by the end of the week.  Tomorrow is the American Thanksgiving holiday -- the day we celebrate all of the land we stole from Native Americans... 

[silnarm]

Looks nice, certainly more streamlined   and a big constructor for the 'Program' class is nothing to worry about I think.

Quick question: Regarding graphics and sound, which can be and are accessed from all over the place, are you just assuming a GuiProgram for now, and leaving it all over the place, or (excuse my wishful thinking) have you funnelled all those calls somewhere so they can be ignored by a dedicated server ?

[daniel.santos]

I'm glad you like it!   As far as the sound & graphics being accessed all over hell & back, I'm addressing some of it, but I can't get it all because I'm trying to get this thing back together.  I have re-added getInstance() functions to some of the classes that call Program::getInstance().getWhatever(), but I've implemented them in the .cpp file so that program.h or gui_program.h aren't pulled into the translation unit. This is slower, but I don't care, it's temporary and carries less consequences than the cost of doing it inline.

However, I've run into another tangent and I'm trying to contain it.  The problem is that GuiProgram::getCoreData() should return a constant reference to the CoreData object.  However, all of the getters for the sound & music objects are non-const.  Why is that?  Because an object of type const Shared::Sound::Sound cannot be played.  This is due to heavy enmeshment of roles in the various sound-related classes.  In Glest::Game::SoundRenderer, when any of the "play" functions are called (playAmbient(), playFx() or playMusic()), you pass a pointer to the Sound object.  The SoundRenderer then calls soundObject->setVolume() -- very naughty!!  Then, stuff in the shared_lib extracts the volume setting from the Sound object when actually playing it.

So the solution is that Sound objects should be passed around as const only.  Other transient objects (the buffer, etc.) can have their state changed all we want, but we should be able to load a sound, play it simultaneously in as many instances of a player was we can and have different volumes for each instance and not have to create a new Sound object and all of that crap.  I think you get my point -- it should be a constant type.  I'll find another mechanism to set the volume prior to sounds being played.

So that's my current side-track, sorry for not having this finished sooner, but I'm getting closer.  As a side note, I noticed that the Vec3f passed to the function to play static sound doesn't map it in the 3d environment!  It just uses it to determine the distance to attenuate the volume.  Since we're not in the 80s any more, we should be specifying the 3d location of sounds to OpenAL -- otherwise, we're throwing out 98% of what OpenAL is supposed to be for!