Print

Please login or register.
1 Hour 1 Day 1 Week 1 Month Forever
Login with username, password and session length

[cerapa]

Why don't you just use SDL_GetTicks() directly?

[DJ]

I'm afraid that if I have a large number of sprites there might be some desyncing, ie they might display different frames when they should be displaying the same frame due to the time it takes to render the ones before. Also, having a clock like this could come in handy for pausing, I'd just stop calling clock::update() while the game is paused. And it might make it easier to convert if I ever decide for some reason to move from SDL to something else.

[cerapa]

Fair enough I suppose. Would need a ton of them for that to happen though, and for people to notice.

Lemme check my own globals...

Code: (CFPS.h) [Select]

class CFPS {
    public:
        static CFPS FPSControl;

    private:
        int     OldTime;
        int     LastTime;

        float     SpeedFactor;

        int    NumFrames;
        int     Frames;

    public:
        CFPS();

        void    OnLoop();

    public:
        int     GetFPS();

        float   GetSpeedFactor();
};

Code: (CFPS.cpp) [Select]

CFPS CFPS::FPSControl;
--------------------------

Code: (InputGlobals.h) [Select]

extern int globalMouseX;
extern int globalMouseY;

Code: (InputGlobals.cpp) [Select]

int globalMouseX;
int globalMouseY;
--------------------------

Hmmph. I think in your case, every addition of the header creates a new clock.

Try putting extern instead of static, and adding clock gm_clock; into the cpp file, or putting the static clock gm_clock; inside the class itself. The second option means you have to write clock::gm_clock instead of just gm_clock when you want to access it.

[MorleyDev]

If that's what you need, why not just pass the delta-time into the functions directly from your update loop, instead of sneaking it in through some global or dependency? If the code's update relies on a delta-time, give it that delta-time. Tell the code to update with a given dt, don't ask what the dt is.

So yeah, the global approach kinda smells to me.

[cerapa]

Contrived complexity: forced usage of overly complicated design patterns where simpler design would suffice.

There is no need. Obviously passing it is much nicer, but I highly doubt he is writing a heavily tested piece of software, where all parts need to be unit tested and not a single person knows what all the code is doing. He knows where the global is, where it is changed, and what uses it. Adding int Ticks into every function that uses it would be 1) unnecessary 2) harder to read 3) a pain in the butt.

[MorleyDev]

It's not a testability thing inherently, I just find globals less readable so would generally advocate avoiding them. My argument against them is that they force the brain to have to stop reading the code and think about something more than what's immediately in front of the developer, namely what this magical off-screen global does. It's a context switch that slows the brain down and makes understanding the function a tiny bit more effort than it needs to be. A parameter is explicitly introduced to a function and obvious, that context switch never happens and so the code is easier to read.

It's like a sentence that See A then returns to the original but still relies on the jumped-to segment. It kinda forces my brain to go "wuh?", have to stop and think. Which wastes time and energy that could be better used. Here a global seems to be an over-engineered solution that does way more than is needed with no real benefit.

So I'd argue the global is the "overly complicated design pattern", and the parameter is the "simpler design". I mean, if the code already uses this global no point going through and refactoring everything en-masse. But if not, and it's a choice between the two, I'd argue the no-global route makes the code easier to both write and read overall.

A: jumps to another part of the page or another page

[Telgin]

Hmmph. I think in your case, every addition of the header creates a new clock.

Try putting extern instead of static, and adding clock gm_clock; into the cpp file, or putting the static clock gm_clock; inside the class itself. The second option means you have to write clock::gm_clock instead of just gm_clock when you want to access it.

This.  It's been forever since I looked up what static does in CPP in this context, but I think it creates a new instance of the variable in each translation unit, which is effectively making a new instance per cpp file.  I'd go with the option of not using static at all here, since it's not needed.  Just declare the variable once somewhere in a .cpp file, then add the extern version in the header for every file that needs to access it.  extern tells the linker that the variable is declared elsewhere so it won't freak out when it sees multiple instances of the variable declaration.

On the whole globals vs. dependency injection thing: I have mixed feelings, especially in games.  I really hate passing around an Engine instance, or an XManager instance all over the place when I know there's only going to be one instance of such things available anywhere.  It feels like overkill to inject them into newly created instances of objects.  Globals are discouraged for good reasons though, so I feel dirty doing it.  I've tried compromising by doing things like just sticking related globals into namespaces, but none of that really feels like an improvement.

The only thing I've really taken away from game design is that it must be impossible to architect a truly well designed game.  Dependencies are just all over the freaking place.

[MorleyDev]

Well here it's more value-injection. I mean, the updating logic rarely needs to know more than how much time has passed since the last update (so what value to integrate by). You'll be trapping the value of the last update constantly with that clock global DJ posted, whilst with a parameter it's easy to work with a fixed time-step, and to experiment with and change that time-step when it's a parameter that comes in from the top, whilst also making it clear to the developer what the consequences of that change will be since it's less likely to propogate to unexpected places.

I mean, the global can exist at the top. std::chrono::system_clock::now() is effectively a global that exists at the top, but that kind of timing isn't really a concern of the code inside the update loop and it seems that it's overly complicating things to push it down (either via a global or injected dependency). A value object (the deltatime) seems the simplest solution. Sure, you could make that global but you lose nothing by making it a parameter, whilst keeping the information a function needs function-local and so gaining clarity.

Games are definitely trickier to 'clean code'. I do think it can be done, for example often you're passing the engine instance around to get at something through the engine so may as well just pass that thing through instead. If what you want is some event queue or data store or renderer or audio interface, you don't lose anything by respecting the law of demeter and giving it that instead of the engine itself. But really a truly perfectly anything is impossible, you need some state at some point after all

I've had some good success using an actor approach (each update handler can be thought of as a subset of actors that always responds to a "timestep" event, each render handler can be seen as a subset that always responds to a "render" event) and even doing that in a multithreaded manner. Helps keep things self-contained and maintain a good separation of responsibilities. But that's only been with small arcade-style games. 

I do tend to just go for a global logger though, logging is definitely the kind of thing I think you can break the rules for. Maybe I'll change my mind on the future with that, but for now xD

[DJ]

I caved in and put the gm_clock into the game class, which passes it to the screen, which passes it to individual sprites in it's render list, which pass to their currently active animation which finally actually uses it to calculate which frame to send back. I guess it's better than using globals, but it just doesn't feel right to me to pass stuff through so many classes that just pass it further without actually doing anything to it. Oh well, I already have the frames going through the same chain (albeit one step shorter) in the other direction, so at least it's consistent.

[MorleyDev]

There's an idea I like, which is to listen to the code. If you find yourself doing something and the code resists, there's a reason for it. Sometimes it means your code design needs to evolve, sometimes it means your idea needs to evolve. So if you're doing something you don't like here, it's often a good idea to stop, step back and consider what that means.

Out of curiosity, why do they need the clock in the first place? I mean, by the looks of things the clock gets the total number of ticks the game has been running for, not the number of ticks the game needs to be updated by. The latter seems like the more relevant information for a frame update.

Look at it this way, let's call the thing that manages sprites your SpriteController. And say it has an update function that takes a dt:

Telling it spriteController.update() is completely hiding the dependency on time, whilst spriteController.update(clock) tells it this code relies on the clock but it's still not clear why or how that'll be used, and it means the update itself is more complicated. spriteController.update(deltatime) immediately reveals that this function will call update on all the sprites by a given deltatime, which is the information that the spriteController would of been pulling out of the clock in the 2nd example and thus that update is simpler.

Also, if I remember correctly the preference in games is for the rendering pass to be a read-only view of the game state, and for animations to be handled by the game logic pass (which operates with a fixed time-step).

[miauw62]

On the topic of bad code, I have recently been rewriting a big underlying system for /tg/station, namely saycode.
(This is mainly just some venting, also note that proc means function)

Spoiler: probably very ranty (click to show/hide)

Lots of bad code. Looooots of it.
For one, the main say() proc was basically just a monolith. There was no way to overwrite anything and if you wanted to add something new it'd have to happen before all the rest was done or after all the rest was done (aka before the checks whether somebody can speak are done or after the speech is sent). This caused tons of bad code, duplicated and half-defective checks and lots of bugs. Not to mention lots of slow typechecks. Also, speech hardly involved the hearer, everything was handled by the speaker, which was very inefficient and akward. The code was bloated, old and slow.

I changed all this by implementing a Hear() proc, replacing the language typechecks by bitflags, and moving the say() proc up a level (defining it at the /atom/movable/ level rather than at the /mob/ level, to solve some bugs where deaf people could hear vending machines talk because vending machines used visible messages to "talk"). I also obviously  split the main proc into seperate functions that can easily be overridden.

But now I've basically hit a brick wall, because of a related piece of code: radio code. Far too complex and weird and hard to understand. Looking at the variables in-game using the View Variables tool (that we had to code ourselves, since BYOND does not provide a debugger), everything appears to be fine. (Note that  this tool can only be used to look at class variables, not at variables in procs, and it does not auto-update either. It's sort of a lacking tool most of the time.)
HOWEVER, the debug messages I coded into the game tell me that everything is not fine, and indeed, it doesn't work. A list turns up empty when VV suggests that it should be fine. All this was caused by me moving the say() proc up a level (requiring me to change some things around by causing some variables to become undefined), not even that. This was caused by me changing the type of an argument in several places so objects talking into radios wouldn't runtime. This caused radios not to work at all.

Even before I changed the type of the argument, radio messages showed up double, and my debug messages pointed out that this is because something in the weird and overcomplicated radio machinery transmits a signal somewhere while it shouldn't.

Did I mention that, for some reason, the radio system is not dedicated to communicating messages? A bunch of machinery also uses it to transmit signals, because it's not an awful idea at all to have several completely unrelated things use the same system, right?

[Maklak]

When it comes to a timer, that you only ever need one instance of in the entire program, I'd either go with a static class (with just static methods and one static variable) or just skip the class and have 2 functions outside of any class and one static (that is, hidden) variable in the same file where those functions are defined. Problem solved, object orientation be damned.

timer.h:
void update_timer();
int get_timer();

cpp:
#includes
static int tmSDL;
void update_timer()
{ tmSDL = ... }
int get_timer()
{ return tmSDL; }

Creating an instance (only one) of such a class, then passing it around as a parameter makes me <unlpeasant> (there is no nice way to say it). Parameters have their place, but they also have an efficiency cost (usually very small) and readability cost. Unless someone believes in functionsal programming, functions causing some side effects or referencing globals / singletons that may have some internal state, is actually OK most of the time. Especially in a small project. Besides, I'd rather have less code than all those "design patterns" anyway. And with this pair of functions you only need to watch closely where update_timer() is called and it should only be one place anyway.

[MorleyDev]

Parameters have a readability cost sure, I've found a good rule of thumb is that if function has more than 3 parameters it's probably doing too much.

I'm not saying pass around the class, I'm saying the constantly accessing the timer from various points in the code seems entirely superfluous in the first place, that it is simpler and easier to just pass the deltatime itself instead of a bunch of repetitious and needless code that deals with the timer. You only need to pay attention to the timer in your game loop, everything else is integration by a time step. The way it seemed the clock was being described as used was that the various subsystems (such as animations) would be using it directly, the point I've been trying to make is that seems to overly complicate those subsystems when really the game loop should be the only place that uses timings directly.

So a game loop could look like (uses c++11 and std::chrono but just mentally swap all the chrono stuff with SDL_GetTicks or your preferred timing library):

Code: [Select]

const std::chrono::microseconds updateRate(10000);
const std::chrono::microseconds drawRate(8333);

const auto startTime = std::chrono::system_clock::now();
auto nextUpdate = startTime + updateRate;
auto nextDraw = startTime;

while (game.isAlive()) {
std::this_thread::sleep_until(std::min(nextUpdate, nextDraw));

const auto currentTime = std::chrono::system_clock::now();
while (game.isAlive() && nextUpdate <= currentTime) {
        game.update(std::chrono::microseconds(updateRate));
nextUpdate += updateRate;
}

        if (nextDraw <= currentTime) {
        game.draw();
                nextDraw = currentTime + drawRate;
}
}


[Maklak]

> I've found a good rule of thumb is that if function has more than 3 parameters it's probably doing too much. 
Heh, I'm using up to maybe 7 under some circumstances (some of them have defaults). But that made me appreciate closures in some programming languages and functions inside functions in gcc.

> I'm saying the constantly accessing the timer from various points in the code seems entirely superfluous in the first place,
> that it is simpler and easier to just pass the deltatime
Ah, yes. It makes sense now.

[MorleyDev]

Heh, I'm using up to maybe 7 under some circumstances (some of them have defaults). But that made me appreciate closures in some programming languages and functions inside functions in gcc.

Well there's a reason I call it a rule of thumb and not a rule or law Always exceptions, often with number crunching or rendering (Like if you have to pass in the texture, source area, position, origin and angle). Parameter objects can help simplify functions sometimes, but the parameters need a clear and obvious logical grouping otherwise you're just making things worse.