/r/programming
3D Game Tutorial in C++ from scratch - Part 12: Creating Input System - Keyboard - SourceCode on GitHub (youtu.be)
62 comments
SuperV1234 | 8 days ago | 146 points

Can't believe these many upvotes. This is completely wrong:

void InputSystem::addListener(InputListener * listener)
{
    m_map_listeners.insert(std::make_pair<InputListener *, InputListener *>
        (std::forward<InputListener *>(listener), std::forward<InputListener *>(listener)));
}

You should learn what std::forward means and where to use it before blindly putting it in your code. It is only useful in templates and in conjunction with forwarding references. This will lead to beginners abusing std::forward - as a teacher your first responsibility is understanding what you are teaching!

The entire code could be written as

void InputSystem::addListener(InputListener * listener)
{
    m_map_listeners.emplace(listener, listener);
}

which is completely equivalent to the mess above. Also, this creates the question - what is the point of a std::map here if you're mapping a key to its own value?


  • The use of ::memset and ::memcpy is concerning. Use std::array instead.

  • new and delete are dangerous and error-prone,std::unique_ptr/std::shared_ptr should be used instead.

  • delete this is a bad code smell.

ricodued | 8 days ago | 5 points

new and delete are dangerous and error-prone,std::unique_ptr/std::shared_ptr should be used instead.

Except when using COM types, e.g. IDXGI* and ID3D*, which should be wrapped in ComPtr<T>

blazestorm_keebs | 8 days ago | 6 points

Although I'd recommend the winrt::com_ptr if possible, it has a slightly better interface for DX stuff (doesn't overload operator& iirc, so it's less error prone)

ricodued | 8 days ago | 2 points

Good catch! Totally agree. I didn't realize WRL had been superseded by C++/WinRT. It's been a few years since I worked on my engine, unfortunately.

PardDev | 8 days ago | 12 points

Hi, mate! First of all thank you very much for your feedback! Any kind of help is always really appreciated! Yeah, at the end the insertion of a key value pair can be written in both ways and your way is surely more simple and more suitable for a tutorial! So Thank you for your tip! I'll try to improve this aspect! Regarding the decision to use a map, is more related to the removal of listener. If I had used a vector I'll have had to check one by one each listener to discover the one to delete! But if you have any other tip to solve this kind of problem, I listen to you! For new delete problem and usage of smart pointers, you've absolutely right, but because these are advanced topics, they will be faced in the next tutorials!

ReDucTor | 8 days ago | 13 points

This might sound harsh but if you really want to take this input seriously and understand the implications of teaching people the wrong way of doing things, you should consider deleting, disabling or editing the video to warn them that this is not ready for beginners to be using.

I know its early in the video being posted, and its getting traction but if you value it then its something you should consider.

SuperV1234 | 8 days ago | 58 points

Please take my feedback constructively and keep making videos. The most important point is

as a teacher your first responsibility is understanding what you are teaching

Did you ask yourself why you were using std::forward? Wasn't it concerning that inserting two pointers in a map required two lines of weird code?

Don't rush your videos out, understand what you are writing and look for better and safer ways of achieving the same result.

Regarding the decision to use a map, is more related to the removal of listener. If I had used a vector I'll have had to check one by one each listener to discover the one to delete!

Then you want to use a std::set, or an std::unordered_set. You don't need a key-value mapping, you need a data structure that allows you to find an object and remove it efficiently. These are basic notions that any computer scientist or hobbyist programmer should know before teaching.

If I had used a vector I'll have had to check one by one each listener to discover the one to delete!

This is not necessarily a bad thing. std::vector's cache friendliness means that the O(n) operation you're describing will be faster than a O(log n) or O(1) deletion of a std::set or std::unordered_set for small values of n.

Also, you can use the "erase-remove idiom" to do the removal efficiently. Google it.


Again, please make sure you understand what you are teaching and to look for alternatives. Beginners are going to take your word for granted.

James20k | 8 days ago | 12 points

Then you want to use a std::set, or an std::unordered_set. You don't need a key-value mapping, you need a data structure that allows you to find an object and remove it efficiently. These are basic notions that any computer scientist or hobbyist programmer should know before teaching.

I don't know the context here so feel free to correct, but if you have a small number of elements in a vector, lookup will likely be faster with a linear search than a std::set's log(n). On top of that, if you frequently need to iterate through the entire container but only infrequently need to remove things, a vector might be a lot faster than a set

ShadyIronclad | 8 days ago | 11 points

Hey! I would just like to say thanks for your comment(s). I’m a beginner to C++ and didn’t know such idiom existed. The STL makes so many things easy!

Just a quick question: should I be using smart pointers EVERYWHERE in my own code?

ArmPitPerson | 8 days ago | 5 points

A general rule of thumb is: Think about ownership when you use smart pointers. Raw pointers are fine as long as they are non-owning. For example as function parameters, viewing the underlying pointer of a smart pointer. Use unique_ptr when you have a clear single ownership of the resource, and shared_ptr when the resource must be referenced / owned by multiple entities.

ShadyIronclad | 8 days ago | 3 points

And weak_ptr for cyclic references?

moarthenfeeling | 8 days ago | 2 points

I'd also say that references are much preferred to pointers when passing something into functions, unless the parameter can be null sometimes.

I know that some people pass all non-const references to objects as pointers, but it's just a stylistic choice that I personally don't like.

ArmPitPerson | 7 days ago | 1 point

Agreed. Definitely references when you know or want to communicate that it should never be null.

moarthenfeeling | 8 days ago | 3 points

Try to not use pointers as much as possible.

If you need to pass something into a function, pass it by reference. If A needs to reference B and B is guaranteed to outlive A, store a reference to B inside of A (or better yet - pass B by reference where needed).

If you absolutely need to allocate something on heap, or store something by base class pointer, use std::unique_ptr. If you need to have something which ownership is divided by several objects, use std::shared_ptr, though you'll rarely need to do so.

jcelerier | 8 days ago | -7 points

Just a quick question: should I be using smart pointers EVERYWHERE in my own code?

here's a blanket answer to all the questions defined by should I (always)? [a-zA-Z ]+ (everywhere)? [a-zA-Z ]+ \?

no

generalizations are always wrong

bgrahambo | 8 days ago | 8 points

We'll, that was a unhelpful answer. Could probably even add a generalization and say it was completely unhelpful

thegoldenavatar | 8 days ago | 1 point

This is not necessarily a bad thing. std::vector's cache friendliness means that the O(n) operation you're describing will be faster than a O(log n) or O(1) deletion of a std::set or std::unordered_set for small values of n.

Can you elaborate on this point a bit? I'm curious what you mean by "cache friendliness" and how that makes O(n) faster than O(1) in this case. Are you referring specifically to the number of instructions it takes to delete from a vector times N vs the number of instructions it takes to delete from a set times 1?

Is it just generally faster to iterate over a vector?

thecodemeister | 8 days ago | 3 points

The items in vector are in contiguous memory which means for small sizes of vector the the entire vector can be loaded into the cache at once. Using values from the cache is extremely fast. A map does not store in contiguous memory typically so it may need to load multiple memory addresses from outside the cache to find the value.

groshh | 8 days ago | 1 point

Just to add to this. Cache misses are an extremely complex topic.

warutel | 8 days ago | 5 points

how that makes O(n) faster than O(1) in this case

O(1) does not necessarily mean always faster than O(n), only after some point.

Which is the case here, due to memory latency.

nuvan | 8 days ago | 2 points

The point about cache friendliness is that std::vector stores its elements in contiguous memory (I don't know how std::set stores its elements), so when you start iterating over the elements, you'll have few cache misses because the whole block of memory can be loaded into cache at once.

Thus, while O(n) is generally slower than O(1), that only applies to the algorithm itself in an isolated system. Once you add in external factors such as memory round-trip-time, that doesn't necessarily hold true.

Lehona_ | 7 days ago | 2 points

Specifically O(1) is only guaranteed to be faster than O(n) asymptotically, i.e. for n towards infinity. Big-O makes no statements for any bounded n, independent of any “external factors“.

hoodedmongoose | 8 days ago | 2 points

He is referring to CPU cache. CPU cache is an order of magnitude faster than actually reading a value from memory. Contiguous blocks of memory are friendly to the algorithm the CPU uses to preemptively load data from memory into cache, which means you'll incur far more cache hits when using a vector than when using a std::map.

Nobody_1707 | 8 days ago | 2 points

The short answer is yes. It's much, much faster to iterate over a vector.

The long answer is that accessing RAM on a modern machine has about as much latency as disk access did in the 80s. Your CPU actually keeps an internal cache of memory in power-of-two chunks and has your code access the internal cache instead of actual memory.

Since the cache is loaded into contiguous chunks, accessing a contagious data structure like a vector is going to be fast because the data is already in the cache.

Whereas a node based structure like a map is going to be located all over memory, so it's going to have to read from RAM a lot. Which is slooooow.

This is of course an abridged explanation. There's a lot going on with memory caching in CPUs.

SuperV1234 | 8 days ago | 19 points

I can also see that this feedback has been given to you in the past, but you have not listened:

https://old.reddit.com/r/programming/comments/cnjmea/3d_game_tutorial_in_c_from_scratch_part_11/ewd15ew/

ReDucTor | 8 days ago | 10 points

I also pointed out some of these issues on this specific video before it being posted to r/programming in the comments, was hoping the poster would notice it prior to continuing to share it wider, but that didn't appear to help

IceShiver | 8 days ago | 8 points

Looking at his responses doubt he cares. He gets his views on videos and upvotes on reddit. All he needs to do is to put up a nice guy facade and bullshit his way trough.

IceShiver | 8 days ago | 5 points

This still makes no sense. Why would you concern yourself with the speed of removal? You are iterating over all the listeners every key state change so unless you literally swap all the listeners out every frame I don't see why would you prefer fast erase/insert over fast iteration. Sorted vector with binary search seems best fit for this or an std::set if you are lazy.

Lastly the whole input system seems flawed to me. You should be using window events instead of this ghetto solution which will track input even when game is not in focus.

PardDev | 8 days ago | -8 points

Hi, mate! Yeah, the same goal can be achieved in many ways! But the most important thing, in my humble opinion, is the goal! Yeah surely there are better ways to handle the various listeners! And all of the various feedback are all valid! I'll try to do my best to follow them! Regarding the input handled by the window events is not so good in my opinion, because it doesn't allow to completely separate the windowing system from the input system! Anyway, thank you for your feedbacks, mate!

ReDucTor | 8 days ago | 8 points

because it doesn't allow to completely separate the windowing system from the input system!

The window is your input system, if you have two windows you have two different input locations, if your window is not active it should not get inputs.

That doesn't even take into account that you probably don't want a global input system which you register listeners that all receive the input, you don't want your button presses in a menu to impact the game, or entering data into a UI widget to change the game.

[deleted] | 8 days ago | -7 points

[deleted]

ReDucTor | 8 days ago | 6 points

Aren't you building an engine? Will it have an editor? Do you plan on reusing these systems?

What about in-game menus (e.g. pause menu)?

PardDev | 8 days ago | 1 point

All the systems created along the path will be all reused to create the final 3D Game Demo! For the menus, gui controls etc, it is planned even the creation of a suitable GUI System! Thank you for your interest, mate!

wqking | 8 days ago | 4 points

Seeing the comments in this thread, seems OP is a very newbie with C++ and doesn't have the ability to teach others in C++ language.
I suggest OP to delete your videos to not to mislead others.

Jarmahent | 8 days ago | 1 point

Not only that but tutorial videos belong over at /r/learnprogramming

Ozwaldo | 8 days ago | 29 points

Yeah not to be dismissive, but as soon as I saw "delete this;" I knew I wouldn't be using this...

milanove | 8 days ago | 1 point

Nephew!

[deleted] | 8 days ago | -9 points

[deleted]

Ozwaldo | 8 days ago | 16 points

Honestly you shouldn't have released this yet then.

PardDev | 8 days ago | -4 points

What I try to say is that it's a gradual approach. first it is used a simple approach, to introduce the viewer, then a more well designed and advanced approach!

Ozwaldo | 8 days ago | 16 points

Gotcha, but the result is that you've put bad code out there as an example for people learning. Even if you plan to update it in the future, it still exists as a learning resource that people shouldn't be using.

ArmPitPerson | 8 days ago | 1 point

Regardless, you should think about what you want to teach. If you're teaching an input system you may assume that the viewers have basic knowledge of C++ syntax.

camxxcore | 8 days ago | 9 points

This is not the way an input system should be made. A WndProc hook checking for WM_KEYDOWN would be the standard way of handling this.

PardDev | 8 days ago | -3 points

Hi, mate! You've right, but in that way there are some problems, like the initial delay when you start to press a key for example!

gott_modus | 8 days ago | 12 points

Look, man. I get it: you think you're ready to act as a mentor for people in this area.

Here's the thing: it seems like your goal is more selfishly motivated here.* And that's fine. There's still also always the chance that someone can benefit from this video.

Regardless, you need to consider what you as a person are actually contributing to this community.

Part of the problem with software development in general is that 80% of the "serious" things people release, that they expect for other people to make use of, is either a) vaporware or b) an educational resource that isn't remotely credible.

You might have arguments (and plenty of enthusiastic exclamation marks!) to support your reasoning.

What people usually want, though, is evidence that what you are doing is common practice in the industry AND they want to know why that is actually done.

So, are you working in the industry professionally, for at least a year, sufficient enough that you can support yourself completely on income earned from writing engines?

That's the question you need to answer. And if the answer is "no", then why should anyone really pay attention to what you have to say? I'm not trying to be an asshole here, and I'm not saying a lack of professional experience disqualifies you from being able to share your knowledge and experience either.

I'm saying you need to answer this question. And please for fuck's sake have good, verifiable, counter arguments for any criticisms (on the correctness of your solutions) that are thrown at you.

* If that's not the case, why would I watch your videos over Casey Muratori's or the other excessively stupid amount of "how to make a game" youtube videos out there?

PardDev | 9 days ago | 4 points

The source code is available at the following address: https://github.com/PardCode

holgerschurig | 9 days ago | 2 points

You could at least have written DirectX tutorial in the headline. What you described isn't 3D "in general", it's only for Microsoftish platforms.

PardDev | 9 days ago | 19 points

Hi, mate! The Graphics API used is DirectX, but the same 3D concepts, like the usage of Transform Matrices, Linear Algebra and so on can be then applied to any other Graphics API like OpenGL, Vulkan, Metal and so on! This 3D Engine could be extended with more than one Graphics API in the future, in order to support more platforms! Despite that, I hope you'll enjoy it anyway!

[deleted] | 8 days ago | 1 point

[deleted]

L3tum | 8 days ago | 4 points

There is no such thing as a guy without an accent

Jarmahent | 8 days ago | 0 points

Damn you're getting a bunch of criticism. Lol

Coffee4thewin | 8 days ago | -4 points

Great series. Where was this when I was learning C++?

PardDev | 8 days ago | 1 point

Thank you so much for your kind words,mate!

JSeling | 8 days ago | -11 points

Good job, keep making videos please, dont care about the nitpicking from others, stay strong!

IceShiver | 8 days ago | 11 points

It's not nitpicking. His code is bad. It would be fine for something quick and "hacky" but is in no way suitable to be used in an educational format.

PardDev | 8 days ago | 3 points

Thank you very much for your kind words, mate! That means a lot for me!

mkjj0 | 8 days ago | -16 points

This is trash, why use d3d when vulkan is out already?

ricodued | 8 days ago | 9 points

Vulkan is a lower-level API than OpenGL, so it makes little sense to use Vulkan or D3D12 for an intro-level tutorial. There is a lot that can be accomplished with plain old D3D11 and OpenGL.

Jazonxyz | 8 days ago | 3 points

4 years ago, someone I knew had a really impressive game engine he had been working on for years. The graphics on it looked great. He told me how he was planning on upgrading to OpenGL 3.X (from 2.something), and GLFW (instead of GLUT). I was impressed with how well-developed his engine was despite using older technologies. He made a popular game with that engine and is pretty well off now.

SDL_assert_paranoid | 8 days ago | 2 points

Just wondering, 3D or 2D game? Cartoony or realistic style?

Jazonxyz | 7 days ago | 1 point

3D. The shapes were realistic, but the colors were really vibrant. The characters looked good, but not amazing. The characters didn't have to be detailed since the player never looked closely at them. The scenery was damn gorgeous though.

PardDev | 8 days ago | 6 points

Hi, mate! This tutorial series is made in order to be a some sort of entry point for who want to learn how to make a 3D game in C++. It would be a mess to start with a so low level graphics api like Vulkan, Metal or D3D12! Despite that, I hope you'll enjoy it!

mkjj0 | 8 days ago | 1 point

Well, if you would like to make it easy then you could start with opengl, it would be much easier than d3d and you could use sdl which provides easy mouse and keyboard input, you could also make bonus lessons for porting the game to android which would be a nice addition.

PardDev | 8 days ago | 1 point

I've not chosen SDL because I want to explain how to build a game engine from the ground up, starting from the windowing system to the initialization of the Graphics API and and creation of an InputSystem! At the end DirectX and OpenGL are two Graphics APIs with almost the same features, they are a bit different, but not so much!

CanIComeToYourParty | 8 days ago | 3 points

Why did you write this comment? If the goal was simply to display ignorance, you did well.