Minecraft Clone in C++ // Code Review

Поделиться
HTML-код
  • Опубликовано: 23 ноя 2024

Комментарии • 395

  • @TheCherno
    @TheCherno  2 года назад +76

    Hope you all enjoyed the video! Sorry about Ghost Cherno 👻... have you made a Minecraft clone before? How did it go? Also don't forget to visit brilliant.org/TheCherno to get started learning STEM for free, and the first 200 people will get 20% off their annual premium subscription!

    • @张向乐
      @张向乐 2 года назад

      Minecraft!😆

    • @TheShadoDragon
      @TheShadoDragon 2 года назад +2

      I'd say Ghost Cherno has it's positives sides as well: You can see the code below the camera. ;)

    • @rob-890
      @rob-890 2 года назад +1

      Why do you ask for comments and then not reply? 😋

    • @TL-Edu-r
      @TL-Edu-r 2 года назад

      I want to ask how it is that for the video 3D Game Programming - Episode 4 - Drawing Pixels! when I run it I don't see anything there and I proceed exactly the same :(

    • @greatcesari
      @greatcesari Год назад

      Your tone sounded pretty harsh, considering the maker said they're just learning.

  • @toffeethedev
    @toffeethedev 2 года назад +148

    love higher-level design talks like this, optimisations are usually specific to the 1 program, but knowledge about code architecture will never stop being useful

  • @not_herobrine3752
    @not_herobrine3752 2 года назад +990

    "the only design pattern a programmer needs to know is ctrl + c and ctrl + v" -sun tzu, the art of code

    • @adamhelberg9228
      @adamhelberg9228 2 года назад +64

      Hipity hopity your code is my property -Dani

    • @nakumjay
      @nakumjay 2 года назад +1

      Bruuuuuuuuuuuuuuuuuuuh

    • @ShredBird
      @ShredBird 2 года назад +46

      Sun "Microsystems" Tzu

    • @Highwind_
      @Highwind_ 2 года назад +7

      @@ShredBird This would explain Java

    • @superscatboy
      @superscatboy 2 года назад +2

      Virgin copypaste Vs Chad *macros*

  • @andrewcranston780
    @andrewcranston780 2 года назад +112

    When doing code reviews, its always good to keep personal 'feeling' or code style debates out. "I just don't like this" isn't a good reason, and doesn't go far in explaining why someone should avoid using singleton for the top level Application class. You could instead go the 'singleton is hard to test' route, and explain how you'd eventually want to write unit tests and instantiate the application inside a test harness.. if its designed to be statically instantiated that becomes hard to do.

    • @kukukudoes458
      @kukukudoes458 2 года назад +13

      Point
      It gets annoying when you tell to not do something but don’t point out why

    • @andrewcranston780
      @andrewcranston780 2 года назад +8

      @@kukukudoes458 usually if no actual explanation follows it's because the reviewer is a cargo cult programmer with no original ideas of their own.

  • @vladimirkraus1438
    @vladimirkraus1438 2 года назад +96

    "explicit" is your friend. Unwanted or unexpected implicit conversions can cause a lot of troubles and hard to find bugs. I always use "explicit" unless I am really 100% sure that I want an implicit conversion for that type. Explicit conversion requires more writing but makes the code much more readable and simpler to reason about. It makes the code more type safe and less prone to errors.

    • @agfd5659
      @agfd5659 2 года назад +8

      It's weird that they didn't make explicit the default now that I think about it.

    • @vladimirkraus1438
      @vladimirkraus1438 2 года назад +9

      @@agfd5659 Yes, it is one the many wrong decisions when designing C++. But back in that time the language priorities were different than they are now. They probably believed that implicit conversions allow for more concise code (which they do) and did not think so much about language safety.

    • @doglitbug
      @doglitbug Год назад +4

      @@agfd5659 I wonder if the author used CLIon and it suggested this all over the place

    • @noerlol
      @noerlol 5 месяцев назад +1

      @@doglitbug He did

    • @doubledispatch6620
      @doubledispatch6620 3 месяца назад +2

      Implicit conversions should be reserved for those that are entirely lossless and can never fail, e.g. an integer to a long integer or a fraction class.

  • @migueldejesustavares4168
    @migueldejesustavares4168 2 года назад +93

    You should 100% make your own Minecraft Clone tutorial series. It would be such a good learning resource.

  • @jumpingjoy7689
    @jumpingjoy7689 2 года назад +21

    The reason that there is a lot of "explicit" in the code is that the programmer is most likely using clion, which includes clang-tidy, which alerts you about single argument constructors. it is also why you see [[nodiscard]] aswell

  • @lukefidalgo8154
    @lukefidalgo8154 2 года назад +1055

    I think every programmer that's interested in making games from scratch will always go through that phase where they want to make a Minecraft clone lol

    • @amadeusk525
      @amadeusk525 2 года назад +196

      Minecraft is just a great concept, programming-wise. It's impossible not to have fun while trying to code it, or just trying to understand how one would do so

    • @makichiis
      @makichiis 2 года назад +115

      Recreating Minecraft allows to you to explore many parts of the development process and touches a lot on optimization because of how performance-draining a voxel engine otherwise is.

    • @eucalypt444
      @eucalypt444 2 года назад +2

      @@makichiis theres no voxel engine in minecraft lol

    • @makichiis
      @makichiis 2 года назад +58

      @@eucalypt444 Is this supposed to be joke? Obviously, since Minecraft is engineless, it’s engine is *internal*.

    • @eucalypt444
      @eucalypt444 2 года назад

      @@makichiis wrong. minecraft uses BrownEngine

  • @bloodgain
    @bloodgain 2 года назад +160

    Additional comments (and disagreements with Cherno's comments):
    - For a better understanding of the Application instantiation argument here, just look up why Singletons are considered a bad practice. If you're going to use a Singleton in C++, though, _always_ use the Meyers Singleton (thread safe), which you did. On this count, Cherno is wrong and going against widely-regarded best practice. He is right that it's subject to the Destruction Order Fiasco, but if you insist on a singleton, you live with that. Just don't have singletons that depend on other singletons -- as if singleton wasn't a bad enough design! There is another pattern called Leaky Singleton that avoids destruction entirely, but one, if you need that you have a bad design, and two, if you're going to go through the trouble of learning how make a good, thread safe, leaky singleton, you'd be better off spending the time re-architecting to get the singleton(s) out of your design.
    - While it's an architecture argument, I probably agree with Cherno's critique of your Ray class. However, passing the World into the function like that? That's _exactly_ how you avoid Singletons **and** tightly-coupled code. It's called Dependency Injection, one method of Inversion of Control -- aka coding to interfaces, not objects. Good instinct, if not the best use of it in this case. Still, way better than making World a singleton!
    - Cherno's argument against using explicit is simply wrong. You absolutely should mark all single-argument constructors as explicit. Not doing so can result in accidental implicit conversion, which adds runtime overhead. All such conversions should be made explicitly, usually by calling static_cast or dynamic_cast. Again, this is _widely_ considered best practice.
    - Why is it called main? Why not? This is an application, not a library. It's probably the IDE's default, and defaults are de facto patterns. Plus, it's just a learning project, and it could always be renamed later. Code reviews should not be nit-picking exercises.
    - Class layout is a little bit of a style nit-pick, but in this case, I actually agree. This is the standard layout that other developers expect to find. If you declare it as a struct (public access by default), this signals something a little different to another developer that this is more datatype than functionality, will probably be entirely public, and you can leave off the "public" declaration. Think of these as de facto standards.
    - Cherno doesn't call you out for not separating your code from your declarations, but I will. If you're going to enclose all your class's functionality in one file, it should be a header (.h) file. But still, separate your code from the declaration, either as "inline"d functions below the class declaration or in a code (.cpp) file. The _possible_ exception is simple one-liners like getters/setters that are only an assignment/return, but separate is better even then. This is another de facto standard, and most style guides enforce it.
    - If you have a destructor, either write the copy and move constructors and assignment operators, or remove them with "= delete". If you write any of these, you should write all of them. This is called the Rule of Five (formerly Rule of Three before move semantics were added in C++11).
    - Your "chunks" map is duplicated. Maybe that actually fits your design well, as long as you're not carrying 2 full duplicates of all of them in memory, but it does mean that *the map type is declared in more than one place.* Abstract that out. At minimum, use a typedef, but if "chunks" has responsibilities beyond what unordered_map provides, then it should be its own type with functions. A clue here would be if you're repeating code to operate on chunks in multiple places.
    - Overall, though, great job. Some of it is pretty clever, actually. Maybe read a style guide or two and check out some C++ best practices guides. Scott Meyer's "Effective" series of books is highly recommended, but there are some good free guidelines and talks out there, too.

    • @miguelguthridge
      @miguelguthridge 2 года назад +14

      That was really informative! Thanks for taking the time to write it. I'm gonna be learning C++ properly (messed around with it in high school) next term at uni so it's really interesting reading all the comments on how C++ code should be written from a stylistic standpoint.

    • @Bobbias
      @Bobbias 2 года назад +15

      @@miguelguthridge universities generally end up teaching horrible code style, so it's good to have a handle on what constitutes good code style before heading in :)

    • @andraskmeczo575
      @andraskmeczo575 2 года назад +1

      @@miguelguthridge if you let me add to that the most you can learn is writing actual code, because then you'll encounter problems, look at other people's code, take the best of everything and have a good coding style after a while. Especially if you build a bigger project like a smaller but whole game or a game engine you'll inevitable follow the good practices because it will be easier to use than bad practices (or at least this was true for me)

    • @Astfresser
      @Astfresser 2 года назад +4

      Great explanation and I agree totally, but I think Cherno meant to avoid the singleton just put the object altogether on the main stackframe instead of having it lying around in global scope, which I would very much encourage. You can just pass your world object to every instance in a functional way and never have to worry about threadsafety.

    • @FokinDenis
      @FokinDenis 2 года назад

      Thank's for information!

  • @beastle9end499
    @beastle9end499 2 года назад +40

    I actually do this with the explicit too if the constructor only has one parameter, because I think it's a better design to have everything explicit by default. This is exactly how you actually make every reference const at first, unless you already know you want to mutate it.

    • @pantsoff
      @pantsoff 2 года назад +16

      Yeah, this is very common. I'm not sure why such a big deal was made about this, especially right after he went over how he likes APIs to be clear

    • @bloodgain
      @bloodgain 2 года назад +11

      Both are considered best practices. Keep doing them!

  • @IONATVS
    @IONATVS 2 года назад +22

    I believe Minecraft Bedrock Edition (ie the modern version of Minecraft Pocket Edition, which is available on Mobile, Consoles, and Windows PCs) has a C++ codebase, as opposed to Java Edition (the Original, available on Windows, Mac, and Linux)’s use of Java (obviously, it’s in the modern name). Maybe it was C#, but in any case, Bedrock is already an entire rewrite of Minecraft in an entirely different language.

    • @Originalimoc
      @Originalimoc 10 месяцев назад

      Yes, but most importantly its block algorithm is much more efficient resulting HUGE fps speed up. Not (mainly) because used C++.

  • @h.hristov
    @h.hristov 2 года назад +178

    Hi Cherno, could you create your own series on design patterns? I wonder what your take would be on them. Recommendations / pitfalls.

    • @Ezdiess
      @Ezdiess 2 года назад +13

      He just made a rant over OOP and you expect him to make a video about desing patterns? Highly doubt it

    • @h.hristov
      @h.hristov 2 года назад +5

      @@Ezdiess Haha that was the reason I wrote this comment

    • @spikespaz
      @spikespaz 2 года назад +3

      He's pretty bad at design honestly, this whole ray casting thing in the constructor was explained horribly and he took too long to come to the conclusion it should be in the world class. Ideally it should be on a behavior interface that anything that can cast a ray can implement.

    • @Psykorr
      @Psykorr 2 года назад +1

      This is actually a good idea. Not because design patterns are that omnipotent as many think, but to set up a problem for which a design pattern COULD be a reasonable solution. For example, if you have a program that have different modes, perhaps in a game you have a start menu, some settings, game play, save screen etc. This could for example be done with the state machine pattern instead of a bunch of nestled if-statements. But every body should know that this could be done in many other ordered and 'clean' ways that are not formal design patterns

  • @joelincz8314
    @joelincz8314 2 года назад +8

    It's inspiring to see what people are creating at the same time super insightful to hear an experienced person comment on it. Thank you to both!

  • @sarahkatherine8458
    @sarahkatherine8458 2 года назад +11

    30:46 I'm sorry if this is not Isti's case.
    This "MovementDirection" trick seems to be used by a lot of previous-generation teacher and programmer (or at least in my country).
    The idea is, instead of writing x += 0; y += 1; to move upward, they will create a "movement direction array" like this:
    dirX = {0, -1, 0, 1, -1, 0, 1, -1, 0, 1};
    dirY = {0, -1, -1, -1, 0, 0, 0, 1, 1, 1};
    Now to move upward, they will do x += dirX[8]; y += dirY[8]; because Num8 is up on your numpad (you are at Num5).
    They said it will saves a lot of if-else for all 8 directions, and makes it easier to tract the direction by looking at the numpad and array index.

    • @AntonioNoack
      @AntonioNoack 2 года назад +5

      Nice trick, but has nothing to do with the code in this video 😅.
      Lsti just defined what direction has which xyz coordinates without any tricks, and stored whether the key is pressed in there additionally.

    • @sarahkatherine8458
      @sarahkatherine8458 2 года назад +1

      Yes, Isti's code is not any trick, just pre-defined direction vectors as Cherno said.
      I just borrowed the name because it reminds me of the mentioned trick 😅

  • @DavidSpry
    @DavidSpry 2 года назад +72

    I suspect he's using Clang-Tidy, which is fussy about the use of `explicit` for single-parameter constructors.

    • @bloodgain
      @bloodgain 2 года назад +34

      It's fussy about it because it's a widely-accepted best practice. Linters and other analysis tools should default to best practices, and if you decide to go against them, put the onus on you to disable that check.

    • @DavidSpry
      @DavidSpry 2 года назад +5

      @@bloodgain Indeed, I agree.

    • @Ben-rc9br
      @Ben-rc9br 2 месяца назад

      ​@@DavidSprysonarqube also

  • @propov1802
    @propov1802 2 года назад +10

    16:16
    C++ CoreGuidelines. "C.46: By default, declare single-argument constructors explicit
    Reason To avoid unintended conversions."

    • @neijiagongfu
      @neijiagongfu 2 года назад

      👆thats a good reason, not just a matter of taste.

  • @zxuiji
    @zxuiji 2 года назад +7

    16:07, another reason to put public stuff 1st is to reduce ABI issues when you change something in the private section, if the public stuff is 1st then it will remain in the same ABI spot so long as nothing changes in the public section (or what it inherits from)

    • @alexanderdonets5321
      @alexanderdonets5321 2 года назад +2

      Glad you mentioned that, now I know the name of the general concept (ABI). Recently I've faced only with "binary compatibility" of assemblies in C#, but had a hunch that this is not a language-specific problem.
      P. S. hadn't got formal education in CS, guess that's why I'm learning this in a weird way :D

    • @zxuiji
      @zxuiji 2 года назад +2

      @@alexanderdonets5321 Good to he- er, read, for reference the 2 common terms I'm familiar with for any language is:
      1. API, while I'm sure you know what this term means I'll clarify for those who are not, Application Programming Interface, all the functions, variables and constants that can access of a given library &/or system.
      2. ABI, Application Binary Interface, what an API compiles down to for the computer to understand instead of humans, even the slightest mis-match can cause the computer to not understand where it should look for a given call, variable &/or constant when it is referenced in the compiled down code

  • @ShinQdan
    @ShinQdan 2 года назад +12

    24:20 It's funny the actual Minecraft is written this way in Java as far as I remember - loads of passing the World object down to entities

  • @victornpb
    @victornpb 2 года назад +26

    while I agree with most of the criticisms, I find that a lot in this video comes down to personal preference, and the author didn't do anything that is specifically bad. I think it's important to separate what are "do's and don’ts" from "there's this other way some people prefer", this discrimination is something very important I learned in code review etiquette.

    • @prezadent1
      @prezadent1 2 года назад +2

      *distinction

    • @csisyadam
      @csisyadam 2 года назад +5

      I had the same feelings while watching this video. In a real world environment, colleagues wouldn't like Cherno's "blaming" :)

    • @victornpb
      @victornpb 2 года назад +1

      @@prezadent1 define discrimination: The ability or power to see or make fine distinctions; discernment.

    • @prezadent1
      @prezadent1 2 года назад

      @@victornpb So now you know. You're welcome.

    • @victornpb
      @victornpb 2 года назад

      @@prezadent1 I don’t think you understand. they mean the same thing.

  • @bluustreak6578
    @bluustreak6578 2 года назад +4

    The thing that impresses me the most is how he's able to understand what the code does so fast

    • @temp50
      @temp50 15 дней назад

      Yes, I'm pretty sure he didn't check the code previously and didn't make notes on it. :D

    • @bluustreak6578
      @bluustreak6578 14 дней назад

      @temp50 i guess that is a possibility too... x)

  • @CediCore
    @CediCore 4 месяца назад +2

    I once made a Snake Clone, and it's still growing somwhere out there.

  • @rmt3589
    @rmt3589 Год назад +1

    That's so exciting! Just started planning out my own Game Engine, and you're one of my recently found inspirations! Been watching a lot of your videos sense. To know that I can send it in for you to review, that's super exciting!!! Deffinately have my heart set on that now!
    Btw, you should totally make your own minecraft clone! Would love to see one done with hazel!!!

  • @aaron6807
    @aaron6807 2 года назад +6

    I started learning OpenGl and C++ by making a minecraft clone too. It's interesting to see how others implemented the same systems

  • @Jesus228FBI
    @Jesus228FBI 3 месяца назад +2

    16:50 "implicit constructors can simplify code a lot" - really cracked me up

  • @DemetryRomanowski
    @DemetryRomanowski 2 года назад +25

    I use CLion with CLangTidy and the explicit comes directly from that, I changed mine to not constantly pester me to make constructors explicit.

    • @makichiis
      @makichiis 2 года назад +2

      This is actually the same reason I use explicit constructors. As a general rule of thumb I will declare a one-argument constructor implicit just so CLang-Tidy doesn't yell at me, but I can also see how it would be good for consistency with other one-argument constructors that may benefit from it.

    • @bloodgain
      @bloodgain 2 года назад +11

      CLion and ClangTidy are correct. Cherno is wrong. Making single-argument constructors explicit is considered a best practice. You _don't_ want implicit conversion in most cases, and it can cost you performance even if it's not introducing errors.

    • @makichiis
      @makichiis 2 года назад

      @@bloodgain Does implicit conversion occur even when your type matches the type the constructor accepts? I must admit that I am not as familiar with C++ under the hood as I should be. Thanks for the insight

    • @bloodgain
      @bloodgain 2 года назад +1

      @@makichiis In most cases, yes. Like all things in C++, the answer is "it depends" or "it's complicated". But if the compiler can determine a resolution for a call ("substitution"), in nearly all cases, it will insert that resolution unless it has been instructed not to, like with "explicit". Unfortunately, the way C++ determines substitution/resolution can lead to some weird corner cases and doesn't allow neat tricks like multiple dispatch, but it's also what makes some other neat tricks work, like complex template resolution (see: SFINAE).

    • @makichiis
      @makichiis 2 года назад +1

      @@bloodgain Ah I see what you mean. It's good to *explicitly* (pun definitely intended) declare the constructor explicit since whether the resolution will actually take place when types match is not guaranteed by the standard (implementation-defined), only that "explicit" makes it so that it doesn't. Do I have that right? I also never considered how this is related to SFINAE. There's clearly still a lot I don't know! Thanks again for your input

  • @davidm2.johnston684
    @davidm2.johnston684 Год назад +1

    This is the video that has spent the longest time in my "Watch Later" list. Years.
    Not that I don't clean it regularly, or that it's a place where videos go to die. But this video has survived there for all this time.
    And I finally watched it today. I needed to be ready. Ready to dive again into C++.
    And so yeah, good video! It made me want to try something like this too!

    • @davidm2.johnston684
      @davidm2.johnston684 Год назад

      All jokes aside, I'm a Python programmer, have been doing that professionally for 3 years, in the animation industry. I've watched a lot of courses on C++, including yours, which made me familiar with the thought patterns, and the specific concepts and culture of the language. And of course the syntaxe.
      But every time I tried to start a project, I got stuck with other stuff. With configuring my project and my tooling. I guess I'm trying too hard to have the perfect setup, with an IDE that is well configured, the right build and build automation tools, etc, etc. And I can never understand all that or get everything to work.
      Same if I try to clone an existing project and build it. I've managed to do that for Blender and for Minecraft (with a modding API), but that's because everything has been perfectly automated for every single platform. With other projects, at the tiniest hiccup, I get stuck for whole days.
      Also working on Windows may not have been the best idea, as many projects are actually configured for Linux.
      Yeah, that's why I was putting that off for so long. But I will give it another go sometime. I'd like to write a ray tracer sometime, and I've saved your Ray Tracing series for when I'd get the courage again.

  • @DipsAndPushups
    @DipsAndPushups Год назад +2

    At 24:50 Yes, PLEASE! Those kinds of questions of OOP, questions of design, what class should do what is what I want to know so bad. I would like to watch videos on that topic in fact this very discussion about what the Ray and other objects should know about the world and which class should take care of collision detections is something that I am really interested to know. It is also interesting that you can build projects as big as Minecraft clones in OpenGL despite the fact that your design decisions are bad.

  • @MaddGameMaker
    @MaddGameMaker Год назад +2

    Application::instance() does not do any lazy loading. In C++, all static variables (including local ones) get initialized before main() is called.

    • @cummins6945
      @cummins6945 7 месяцев назад

      The C++ standards disagrees with you here (see [stmt.dcl]), but tbf you might have been thinking of C. However C doesn't do any dynamic initialization for statics and it's compile time only. Another detail is that the C++ initialization-on-enter is required to be thread-safe: "If control enters the declaration concurrently while the variable is being initialized, the concurrent execution shall wait for completion of the initialization." This means the compiler has to insert synchronization there when needed

    • @isodoublet
      @isodoublet 3 месяца назад +1

      Not true. You can verify very easily that it will get loaded on the first call -- it's the entire raison d'être for this particular pattern (Meyers' singleton)

    • @marcsh_dev
      @marcsh_dev 2 месяца назад

      And in fact they have a bool sitting in the initial static data area with the program (this isnt necessarily how they must implement it, just how the particular compiler did).
      I had to track down a "static" getting initialized twice, which ended up being an array being written to out of bounds, and resetting the bool to 0, and so, rerunning the code that set the function local static.

  • @szirsp
    @szirsp Год назад

    15:55 I heard a presenter say on some C++ conference that they changed their mind on public first.
    When I looked into it, I found a good argument against public API first: documentation. Read the documentation if you just want to see how to use it.
    If you want to see how it works, then you open the code, in which case you want to see the data first.
    You want to read from top to bottom, and you need to see declarations first and then their usage.
    So it's not really about public first or private first, it's about fields (member variables) first then methods (member functions). It just happens that in most cases data is private and methods are public.

  • @ale-lp
    @ale-lp 2 года назад

    There are so many cool things you could expand the Code Review series into... General review, optimization, architecture. I'd watch them all even if it's an hour long video per project per sub-series!

  • @samarthtandale9121
    @samarthtandale9121 Год назад +1

    Love this review 🤩 learnt a lot !!! Huge fan cherno, wish I'll meet you some day!

  • @vaanidel2117
    @vaanidel2117 2 года назад +3

    30:54 A camera tutorial would actually be really nice 😅

  • @ronin2963
    @ronin2963 2 года назад +1

    It's really Kool that you are doing this. It is one of the most positive aspects of the coding community

  • @ruddymolina7044
    @ruddymolina7044 2 года назад +1

    Master you have provide us with new knowledge... thanks master.

  • @randyprime
    @randyprime 2 года назад +7

    that studio lighting is SEXY AF

    • @austin-m8r-w6d
      @austin-m8r-w6d 2 года назад

      Thank you Randy (love you btw, come back soon but only if you feel like it)

  • @LegoDinoMan
    @LegoDinoMan 2 года назад +2

    I’d love for you to turn it into an optimisation series! Teach me the ways of speed

    • @atlantic_love
      @atlantic_love 2 года назад +1

      I wouldn't, and that's not what he's about. There are optimization channels you can frequent.

    • @LegoDinoMan
      @LegoDinoMan 2 года назад

      @@atlantic_love Would you recommended some please?

  • @charlestheanimator1676
    @charlestheanimator1676 2 года назад +3

    15:40 I would agree with you to that point about class labels for private and public. That said I have pretty intimate knowledge of the Unreal Engine and their code is riddled with header files that have multiple private, public and protected sections. Upwards of sometimes 3 of each per class. AND, every single class has private data members in the top 'default' private section (of which is not labelled private).

    • @amayesingnathan
      @amayesingnathan 2 года назад

      I like to have multiple private, public, protected for members and methods and operators. This gives a clearer boundary between functionality and state in the class. It only ever really ends up applying to private/protected because I almost never have public data members.

  • @mike200017
    @mike200017 3 месяца назад

    The reason for using explicit constructors for single argument ctors is mostly to preserve strong type safety in function calls. Imagine you have a function that used to take a "string" parameter and later on you change the parameters and it just happens that where it used to take a string it now takes a "Persistence" object. Without the explicit ctor, this will still compile and might lead to a really annoying bug.

  • @daithi007
    @daithi007 2 года назад +1

    14:00 Moved the private data members to the bottom of the class implementation.
    You're complaining about it being first but you're looking at the implementation you're not looking at the header.

  • @stephenyork7318
    @stephenyork7318 2 года назад +1

    One of the most important things I've learned over the years with OO is to STOP inheriting and making classes do too much. It's so much more manageable to to have data classes which represent a thing (noun) and have the behaviour of that thing represented by other classes, which typically do only one thing and do it well. I don't actually mean never inherit, but the ability to inherit generally leads to problems and it's a fine art to know when you genuinely need to do it. But anyway, dependency injecting (the pattern not and IoC container) following SOLID keeping clases super simple was a revelation to me.

  • @Garfield_Minecraft
    @Garfield_Minecraft 8 месяцев назад +2

    Better than official bedrock version

  • @Ezdiess
    @Ezdiess 2 года назад +2

    What a brilliant brilliant plug

  • @NewPlayer4U
    @NewPlayer4U 2 года назад +1

    TheCherno!
    I remember your C++ series with these awesome memory checks!
    Smart Pointers was just BLAST!
    Where are those razors? ;]
    How are you even doing Cherno?
    Do you live well?
    Everything alright?
    Maaan, I wish we can drink vodka someday together ;]
    Thanks a lot for your time, I really appreciate it!
    You're the best!

  • @szirsp
    @szirsp Год назад +1

    26:10 I think that Ray utility class might just needs a rename (something like WorldBlockColliderRay), or maybe moved into the World class.
    I mostly agree with Cherno here but I kinda disagree that the player and ray necessarily exist within the world. Minecraft has multiple dimensions/worlds. The ray belongs to the player. It can exist outside of a world (if you want to have a character creation/customization menu/screen... but then you wouldn't need the ray), or it can exist in multiple worlds simultaneously: like stepping or looking trough a nether portal or end portal (if you wanted to make them see-through, I saw a mod for that in MC). But you can achieve these as if the player existed in a void world or in multiple worlds all at once, call the world's ray collider function on multiple worlds.
    You could pass a collider object to the Ray and implement the world's collider, but the result is probably the same, that world specific code doesn't belong to a generic Ray class. You probably should ask the world for ray collision result, rather then a generic looking ray dig into the world internal structure.
    (The indirection makes this slightly less efficient, but you can still use this in the constructor.)
    I get why the work is done in the constructor, it's more efficient, the results take less space to store than the inputs, and called less frequently.
    I don't know if the world is holding only block data, but at some point you might want to implement entities (mobs, maybe chests), then ray collisions with those might be separate from world, and that might be a reason why not to implement this in the World class. Still Ray probably should receive a list of colliders (world's, entity's) and call those.

  • @kech-agmaio8620
    @kech-agmaio8620 2 года назад

    love the chill background music

  • @TechEreb
    @TechEreb 2 года назад +2

    I really like this kind of video. It helps me learn better coding practices.

  • @CodAv123
    @CodAv123 2 года назад

    Regarding the missing header files: I often see people using CMake not adding them to the targets, because technically that's not really required for building and CLion (which the author probably used) will show the directory structure instead of just the target files as VS does.
    Adding the headers is advisable for multiple reasons though. CMake will use all files added to the target - even non-code files - to determine if a target is out of date, depending on modification dates. So not adding headers might not retrigger a recompile of a target with some generators/build tools. Also adding headers will make them show up in project-based IDEs like VS. In addition to that, one should also set some additional CMake properties to properly sort targets and files into filters/subdirs to make them easier to browse.

  • @crusaderanimation6967
    @crusaderanimation6967 2 года назад +16

    1:35
    Artists:
    Artist1: NOOOOO yOu stol My art !
    Artist 2: No it's my ART ! !
    Programmers:
    Programer1: I stole your code lol
    Programer2: This part of code isn't even mine code XD

  • @m4rt_
    @m4rt_ Год назад

    2:30 Jonathan blow is taking making it from scratch to the extreme.
    He is working on a game, in a programming language he is making, where it has no separate engine, just the game.
    Though I think he did copy over C++ code from his earlier games that didn't have a separate engine either.

  • @danieloberhoff1
    @danieloberhoff1 2 года назад +3

    I disagree with the explicit constructor criticism. You can accidentally create an instance, even if the function has const& args. especially if the coinstructor takes only string.

    • @TheCherno
      @TheCherno  2 года назад +1

      You can’t really accidentally create a class from a string, since that would take two conversions (const char* to std::string to class). In my experience, in most cases where a conversion is possible (for most types, not all), the conversion is the intended behaviour

  • @NordicFrog
    @NordicFrog 2 года назад

    18:16 that is a Linus Tech Tips level segway right there. Just perfect.

  • @monetlarry8400
    @monetlarry8400 2 года назад

    Thanks, Cherno! I learned a lot from this episode!!

  • @Throckmorpheus
    @Throckmorpheus 2 года назад

    From modding Minecraft, I'm pretty sure the thing of passing World into functions that feel like they should belong to World is being copied from the way actual Minecraft structures itself.

  • @joschemd
    @joschemd 2 года назад +1

    I disagree on the comment of having the main app being static vs instanced.. the context is predictable there should only be one and its just splitting hairs and really just a case of personal preference.
    And while you give alot of great points and recommendations almost all are what we used to call religious views.. they don't actually make the code better but they make it easier for the reviewer's ease of understanding.
    Re: ray scenario.. you're spot on. There's a reason game engines have a base type exactly for the lack of forcing a specific type, however the more you abstract the more work you have to do for specifics.. a trade off between versatility and utility.

  • @Erock634
    @Erock634 2 года назад

    Really good advice! I love your coding style

  • @emi6aston
    @emi6aston 2 года назад +2

    Idk why, but, Ghost Cherno made me have to pay attention to the code D:

  • @pmcgee003
    @pmcgee003 Год назад

    Passing state (the world) into a function *is* a functional approach.

  • @DeathAtYourDoorStep
    @DeathAtYourDoorStep 2 года назад

    Love the code review videos Cherno!! Keep them up thank you

  • @stephenkamenar
    @stephenkamenar 2 года назад

    i like the performance reviews.
    the main point of writing your own engine is performance.

    • @thepurplepanda4
      @thepurplepanda4 2 года назад +3

      Or learning what happens under the hood.
      I'm not making an engine, but I am making a graphics program. My purpose is purely to learn and show a project for potential work.

  • @abhinavjha3082
    @abhinavjha3082 2 года назад +5

    Could someone tell me which theme Cherno is using? I love it

    • @cole-nyc
      @cole-nyc 2 года назад +2

      It's a theme you get with a payed VS extension called VisualAssist. You can, however, recreate this theme yourself with a color-picker on the colors you see in the video

    • @abhinavjha3082
      @abhinavjha3082 2 года назад

      @@cole-nyc ooh thanks bro

  • @gravenviolet
    @gravenviolet 11 месяцев назад

    Thank you for giving me a glimpse of how to make a minecraft bedrock

  • @yoyo12345
    @yoyo12345 2 года назад +2

    waiting for java code reviews :)

    • @svens3722
      @svens3722 2 года назад +2

      then he have to do it in a pool that nobody sees him crying.

    • @yoyo12345
      @yoyo12345 2 года назад +1

      @@svens3722 sad, why do people hate java

    • @svens3722
      @svens3722 2 года назад

      @@yoyo12345 i started with c and c++. After that i got in school java and i was confused where all come from and i didnt liked it how it worked. but i have to admit that this opinion was kinda of my "beginner noob programmer" thinking builded.i didnt understand it fully. nowadays i think its quite handie and i would like to step into if i have to. but with my private stuff i prefer other languages.

  • @jcx5750
    @jcx5750 2 года назад +35

    "I can do Unreal Engine 5 lumen tech in three months if I was free of work"
    -Does a code review of minecraft in c++

  • @frydac
    @frydac 2 года назад

    wrt the ray depending on the world: this code is highly, pretty sure unnecessarily, coupled to the world class. A ray class is normally something you want unnittests for to make sure it works correctly and it enables you to change/optimize the implementation without worrying about breaking it's functionality. When you write lots of unittests, and you should, then you notice that setting up a world class just to test the ray is quite annoying. If you ever start a new game and need a ray there, this class is also not usable as you probably want a different world (though this ray may be really specific to the minecraft block layout of course). I would strongly recommend trying to write unittests and think about the testability of building blocks like a ray. It will make it relatively easy to make your code less coupled, and thus more maintainable and reusable. This is kind of the idea behind TDD, imho it really works to think about testability all the time when designing, not necessarily writing tests first, though in some cases that works very well too.
    Same for the singleton, it really undermines testability: you want different testcases to be completely independent, but a singleton keeps existing between testcases so it makes it more error prone as you need to make sure to reset the singletons state between testcases.

  • @kariakichernocherno2274
    @kariakichernocherno2274 2 года назад +1

    Hi cherno this is a great podcast love you man!

  • @OperationDarkside
    @OperationDarkside 2 года назад +1

    I'm also doing a Minecraft clone, but it's nearly not as tidy as this guy's code and has a lot less features.
    I startet like 3 years ago and have less features than he does xD
    However, mine has more evolved into more of a experiment lab for graphics stuff. Just figuring out how to write a decent blur renderer took months and recently I ported the app to C++ modules, which ate another 2 weekends.
    And I'm guilty of getting lost in code optimization, despite running at 120 fps at 2% CPU load. GPU has been strange though. when I implemented a way to reduce vertices per chunk and draw calls in general the GPU usage jumped to 50% oO

  • @VioletGiraffe
    @VioletGiraffe 2 года назад +6

    You're just teasing us with all these videos that are NOT about ray tracing, right? :)

  • @Spongman
    @Spongman 2 года назад

    yeah, look at the asm for local static instance. it has a memory barrier around it, which is going to kill multi-threaded performance, especially on something so central.

  • @teamcreative8302
    @teamcreative8302 2 года назад

    I was waiting for this to happen!

  • @awwastor
    @awwastor 2 года назад +3

    25:00 You can just do cast_ray(Ray, World, ..context) to be completely neutral

    • @feschber
      @feschber 2 года назад

      But then this function would need knowledge about World, which only World itself should have.
      So imo having a World::intersect(Ray &ray) function is still the best approach (and our professor did it this way in a university assigment as well)

    • @awwastor
      @awwastor 2 года назад +1

      @@feschber why should only World have access to itself, but Ray not?

    • @feschber
      @feschber 2 года назад

      @@awwastor its not about access, but why should Ray need to know anything about world? That just complicates things. Ray doesn't _need_ to know anything about the world.

    • @jan-lukas
      @jan-lukas 2 года назад

      ​@@feschber my rule of thumb is: when two (or more) headers include each other, I've made a mistake and should move some things around

  • @diamondtulip6186
    @diamondtulip6186 6 месяцев назад

    I liked the part ray vs. world. I always hated object oriented design because of seeing such upside down thinking so often in the code. Making the ray casting the property / ability of the world is very natural, stuffing the whole world inside the ray via constructor feels so upside down.

  • @TheRealMangoDev
    @TheRealMangoDev Год назад

    "Application is such a like important part of your application that like uh like why not you know just kind of create it here" - Cherno 2022

  • @Wilksey37
    @Wilksey37 2 года назад +2

    If you want a code review challenge, review the BUILD engine (Duke Nukem 3D) code....

    • @AntonioNoack
      @AntonioNoack 2 года назад

      Oh well, then he could review Unreal Engine, Blender or Godot as well, but that's waaaayyyy too much.
      or my engine, but that's still 100k lines 😅

    • @Wilksey37
      @Wilksey37 2 года назад +1

      @@AntonioNoack BUILD engine is notoriously bad and convoluted though...If you want him to review you engine then send it to him.

  • @pinguluk1
    @pinguluk1 2 года назад +1

    What's that menu at 7:42? It's a library that you can implement in C++ GUI projects? I've seen it in other C++ projects as well 🤔

  • @mobslicer1529
    @mobslicer1529 2 года назад +7

    In CMake, you have to explicitly include the header files in the targets

    • @Tachi107
      @Tachi107 2 года назад +2

      Or, even better, you can use target_sources(FILE_SET)

  • @Highwind_
    @Highwind_ 2 года назад

    Beware that, when dealing with 'new', you yourself not forget to 'delete'... for when you dynamically allocate memory. A segmentation fault looks back.

  • @johnmccain3050
    @johnmccain3050 2 года назад +1

    @The Cherno When you showed your Hazel code how you set up the "Application". Why didn't you use a smart pointer to wrap your "Application" object? you explicitly have to call "delete app".

    • @user-dh8oi2mk4f
      @user-dh8oi2mk4f 2 года назад +2

      It doesn’t make any difference. There’s only one logical place to delete the app.

  • @moonhowler667
    @moonhowler667 5 месяцев назад

    Four bits is a straight line, my dude. Or a 4x4 flat square.
    0 1 2 3
    4 5 6 7
    8 9 10 11
    12 13 14 15
    And a chunk is 16x16x256. Assuming you're not storing data for ANY cube not part of the surface mesh, and not storing the state of air blocks, you'd at best end up with 16x16 cubes, not 16. So that's 64 bits. Then you're not accounting for underground areas, which add more data to a chunk.

  • @DemsW
    @DemsW 2 года назад

    If you ever find the will to make a minecraft clone I would love to follow that, and I don't think it would take you much time tbh.

  • @alfredli4032
    @alfredli4032 2 года назад +1

    Cherno I like this review very much, learnt alot. by the way can you do a hazel version minecraft

  • @lunaerx2017
    @lunaerx2017 2 года назад +1

    Hi Cherno!
    I want to ask you to make the review of WebGPU technology.
    I see it has some potential in the future not only for web, but also for desktop because it provides an abstraction over native APIs like Vulkan, Metal and DirectX.
    Unfortunately, I don't see the full picture of it because of a lack of experience working with graphics in general. So I would really like to hear thoughts and opinion from you as a very experienced graphics software engineer.
    Hope you'll see this message and eventually make the review!
    Thank you.

  • @skeleton_craftGaming
    @skeleton_craftGaming 2 года назад

    The only program that has ever not been copied in a given language is the compiler/ interpreter for that language

  • @adamhelberg9228
    @adamhelberg9228 2 года назад +1

    I have seen over a few videos that you dont like singletons, is there a reason ?, does it make it slower or use more memory?, I'm still learning so knowing might help for future.

    • @AntonioNoack
      @AntonioNoack 2 года назад +1

      Singletons can be bad for modularity, because you can only have one of them.
      On Android you're additionally told to avoid static references to your main UI class, because the system cannot garbage collect it then, when the classes shall stay in memory (e.g. so the app can restart faster).

    • @bloodgain
      @bloodgain 2 года назад +3

      Basically, it's a global that _forces_ you to only ever have one instance of it.
      Because it's global, it makes testing harder, because it's an undeclared dependency. Better design would be to declare all dependencies as parameters (either at construction time or use time). This is called Dependency Injection or Inversion of Control. While some things conceptually are globals, actually using global references should be done very sparingly, and avoided whenever possible. But this goes for _all_ globals.
      But worse, because it _enforces_ a single instance, it takes away the programmer's control over its lifetime. Thus, it has to be created at startup, live after shutdown or a least until static destruction time (leading to the Destruction Order Fiasco problem), or often both. This is a massive drawback that can be solved by simply not having a class control its own creation and destruction time.
      So, if you do decide to use a global, avoid making it a singleton pattern. You can have a global if you have to, fine. You can have only a single instance, also OK. But don't push those into the design of the type and adopt more downsides than necessary.

  • @brightwebltd2864
    @brightwebltd2864 2 года назад +1

    Cherno! Check out 4D Minecraft - that’s some crazy game design and math!

  • @matyasnyilas
    @matyasnyilas 2 года назад

    Woooaahh i didn't expect a Hungarian project! Let's goo! 🇭🇺

  • @webinno
    @webinno 2 года назад

    Please make your own minecraft clone (in C++) and review briefly for beginners like me if that's a reasonable thing to do.

  • @me-ld2gq
    @me-ld2gq 2 года назад +1

    Ayyo someone hungarian. Finaly.

  • @zxuiji
    @zxuiji 2 года назад +2

    as for constructors vs destructors, I like to put the destructors 1st, even in the implementation file, it makes it easier to know what you need to initialise in the various constructors since I would normally just start writing the next constructor directly underneath the destructor so I can quickly reference it, also since the destructor has a shorter api it just looks nicer to go from short api to long api

  • @BartKus
    @BartKus 2 года назад

    16:16 explicit single-argument constructors are suggested by Google's style guide. "google-explicit-constructor" clang-tidy check can enforce this.

  • @hehe-tw6jl
    @hehe-tw6jl 21 день назад

    made in heaven 7:55

  • @DominikGuzowski
    @DominikGuzowski 2 года назад +2

    Well now we need a HazelCraft 😂

  • @SerBallister
    @SerBallister 2 года назад +2

    11:15 Declaring "Application app;" in main() will allocate that class in stack, which might not be good idea if there a bunch of other classes, arrays and other large objects in there.

    • @TheCherno
      @TheCherno  2 года назад

      Yeah but his Application class was like 30 bytes, and in my experience you’d probably never have an Application class that was stack-heavy in most projects

    • @TheCherno
      @TheCherno  2 года назад

      @@SerBallister perhaps on certain systems, but otherwise I would just heap allocate most of those heavier classes within the Application class

    • @SerBallister
      @SerBallister 2 года назад

      @@TheCherno It's asking for trouble :)

    • @SerBallister
      @SerBallister 2 года назад

      @@TheCherno Yeah, either that or static/global, since I'm absolutely certain I only ever need one instance of that class

  • @AnythingSoupVODS
    @AnythingSoupVODS 7 месяцев назад

    The software development cycle, Steal - Rewrite - Learn, wash rinse repeat

  • @Caellyan
    @Caellyan 2 года назад +3

    vert_lighting (ln 46) of shader code can be optimized a lot with regards to number of instructions generated. min(max(..)) screams "replace me with clamp", and in this case the glsl compiler wouldn't replace it with min(max()) as the upper limit is a variable. Sending less data to shaders is good, but in case of a MC clone this might be unnecessary as the world doesn't change that often - so computing positions every frame in a shader is increasing amount of work that needs to be done per frame. This depends though on how much work you need your shaders to do I guess.

  • @Андрюхаслазерки
    @Андрюхаслазерки 2 года назад

    Good. Very good video.

  • @abdelhaksaouli8802
    @abdelhaksaouli8802 9 месяцев назад

    game engine are made to facilitate for BIG studios to make a lot of games and focus on game design rather then coding that's the purpose of game engines

  • @zxuiji
    @zxuiji 2 года назад +1

    20:33, I would not check for air, instead I would just have a null block that is treated as air whenever it's referenced, something like:
    typedef struct _FACE { float bends; vec3 emits; } FACE;
    typedef struct _CELL { FACE faces[6]; } CELL;
    ...
    CELL *cells = calloc( cell_types_count + 1, sizeof(CELL) );
    ...
    fread( cells + 1, cell_types_count, sizeof(CELL), cell_types_file );
    ...
    while ( ray.dist < 3.0 )
    {
    ...
    cell = ...
    face = cells[cell->type].faces + (...)
    ...
    light += face->emits
    angle += face->bends
    ray.dist += ray_jumps_normalised_dist
    ...
    }

    • @zxuiji
      @zxuiji 2 года назад +4

      @@richardlyman2961 Did I ask if you cared?

    • @yearswriter
      @yearswriter 2 года назад +3

      @@richardlyman2961 the f dude is Just sharing very relevant opinion, lol, at least argue with proper argument on the subject

    • @zxuiji
      @zxuiji 2 года назад

      @@richardlyman2961 So? Normal people start conversations with anything they feel like talking about, do they ask if the person they started speaking to cared? No so I suggest you stop being a sheldon and quit complaining.

    • @Bonnie20402
      @Bonnie20402 2 года назад

      @@richardlyman2961 When I ask for you to ask, that’s when you ask ;)

    • @adiaphoros6842
      @adiaphoros6842 2 года назад

      @@richardlyman2961 No one asked you to live. And no, your parents don’t count.

  • @Kisamee2570
    @Kisamee2570 2 года назад

    Wooww, cool man, whos make the code is cool people. Amazing.

  • @thelowendstudio
    @thelowendstudio 2 года назад +2

    Alternate title: Minecraft C++ mining

  • @MirrorsEdgeGamer01
    @MirrorsEdgeGamer01 2 года назад

    The explicit constructor is a JetBrains thing.

    • @bloodgain
      @bloodgain 2 года назад

      The explicit _single-argument_ constructor is a best practices thing, and thus encouraged/enforced by many tools.