Podcast #128: We chat with Kent C Dodds about why he loves React and discuss what life was like in the dark days before Git. Listen now.

Hot answers tagged

12

A couple things I would have done differently, plus some other broader advices: Renderable2D seems to me is representing a single sprite, each sprite being a quadrilateral. In that case, you really don't need a variable amount of texture coordinates, but rather just 4. No need to use a std::vector in this case and force a heap allocation. Just use a plain ...


12

General Structure: Indentation. Be consistent. It makes the code really hard to read when this is all over the place. Also add a layer of indentation after every block opens { and remove it when the block closes }. Memory Management You should not be using new/delete. Most of the time automatic variables are preferred. And when you want dynamically ...


10

Disclaimer: This is the first time I have ever really looked into using OpenGL Keep in mind that my review may contain code that is not fully included in the question (such as the simplified main() function). Bugs I found that when I tried to resize the window, I would always crash the program. That wasn't very fun, so I set out to fix that first and ...


10

Refactoring code structure As of right now, you have: A struct named Object. A global std::vector<Object> of Objects. A function named attraction, which (I assume) calculates the attraction between two separate Objects. A function named render which (I assume) renders all of the objects in bodies. While this structure works fine, it's somewhat rigid ...


9

Here's what I came up with. I'm not sure if this is better or worse, but it's less code. Feel free to review this instead of the code in the question (or both). Some extra definitions get around the upper/lower case problem mentioned in the question: #define LUA_Tlightuserdata LUA_TLIGHTUSERDATA #define LUA_Tstring LUA_TSTRING #define LUA_Tnumber ...


8

Since I'm not a JS guru or anything, I'm going to give more of a style review, rather than a review of the code, but if there is anything I see, I'll try to point it out. First off, you have some odd comments, which I assume are to describe the return values of certain functions, e.g, // -> bool. While there are many different commenting styles, the one ...


8

I concur with @glampert's suggestions. I have a few questions of my own. Naming What is the purpose of the Submit() method? What is it submitting? (I don't see any calls to OpenGL to submit vertex attribute data, for example.) Also, it appears to update the value of an internal pointer, m_Buffer. What is it a buffer of? Its name should reflect what it ...


8

You are leaking the Vertex Buffer handles! In your create function you allocated two Vertex Buffer Objects (VBOs), one for colors and one for positions, but you never store them anywhere, so your code will leak that memory, since those handles are no longer accessible after newTriangle returns. You should augment your GameObject structure to save those ...


8

A few general comments Overall, your code has a very "90's" look-n-feel to it. I say that because old codebases and your code tend to have two things in common: Heavy use of raw pointers and manual memory management (new/delete). (ab)use of try/catch blocks. Number one is a no-brainer to fix. Get familiar with smart pointers and modern resource management ...


7

Not really sure why this line is neccesary. print("Fuck") Unless "Fuck" is Russian for something, this should be removed. In addition, using % for string formatting is deprecated. One should be using str.format instead. str.format supports normal, positional, and named parameters. Here's an example: # str.format without positional or named parameters ...


7

Tips on coding style: Names starting with an underscore _ are reserved for use by the C++ implementation and should definitely be avoided. I believe that you have defined the types _Uniforms, _Meshs, _Programs and _States with an underscore in the name to make them internal/private to your library. If this is the case, then remove the starting _ and place ...


7

First of all, good for you for learning how to write a proper makefile. Few people are any good at it, and it's a really fundamental tool. Even when nice tools such as CMake are used, it's very useful to understand how to write one from scratch. With that said, here are some things that may help you improve your makefile: Prefer Makefile to makefile The ...


7

Your implementation is very OOPy and I'd call it a good rendering of the builder pattern. Your usage example makes for a nice "fluent" API, BUT, I would personally not buy it. The reason why 99% of the implementations out there are just reinventing OpenGL in a OOP way is because everyone takes the sorter path of a bottom up approach. They look at the API ...


7

Variable initialisation in condition Both here: uint8_t rv; if ((rv = initialize_glfw())) DIE("%s%" PRIu8 "\n", "initialize_glfw() failed with code ", rv); here: GLFWwindow *window; if (!(window = glfwCreateWindow( image.width < screen_width ? image.width : screen_width, image.height < screen_height ? image....


6

I know nothing of OpenGL and what a correct implementation of a VertexArrayObject should look like, but I do have a couple of observations: I like that the ID private field is readonly, ...but its naming breaks the naming convention for private fields, which should be either id or _id - looking at the other private fields _id would be more consistent. I don'...


6

I'm no Lua expert, but I can at least give you a pointer (no pun intended) about your C code. Your get_pointer function looks like it works fine, but it could be made even more concise! Lua provides a luaL_checkstring function that combines luaL_checktype and luaL_tostring. Therefore, you could just do this: int get_pointer(lua_State *L) { ...


6

A few general comments, as I know nothing of opengl or lua. Why make gltext_t contain pointers to structure that must be allocated individually, rather than just containing the structs (i.e. omit the '*')? Gltext is an alternative type name that avoids using the reserved _t suffix. When you do allocate structures using malloc you should omit the casts. ...


6

Declared types by value when possible: Dynamic memory allocation places an overhead in both the machine and the programmer, so it should be avoided when possible. Most fields in the gltext_t struct can be declared by value: typedef struct { // Pointers first text_buffer_t * text; vertex_buffer_t * vert; // These are all floats ...


6

Skimming through the code, a few items jump out: Why assume shader sourcecode is stored in a HTML container? Split the creation and reading logic into two objects or at least methods. Consider using gl.getAttribLocation and gl.getUniformLocation instead of prebinding locations (if anything, your setupcode is not longer required). Consider using type ...


6

Why don't you simply set the texture coordinates properly and have a "pass-thru" shader that just samples the texture and nothing else? For example, if you're using glVertexAttribPointer() with an array of vertices and texture coords, make sure that the texture coordinates you send have the proper tile offset. Something like this: const int numXTiles = 8; ...


6

Yes, there are a couple things you can simplify and improve in your shader code. width and height are constants, so you can either use const or #define to make sure they are resolved at compile time. Since scalarX and scalarY only depend on width/height, those can also be compile-time constants. I suppose x and y are set to constants values in your code for ...


6

I'm just going to review texture.py. There's no documentation. What kind of thing does an instance of the Texture class represent? How am I supposed to create and use one? What arguments do I pass to the load_from_data method? What does the get_data method return? Do I have to call the methods in a particular order for things to work? And so on. Image ...


6

I see a number of things that may help you improve your code. Fix the bugs There are a couple of bugs in this program that should be fixed. First, the include guard in Window.h is missing an #endif, but that's an easy fix. Second, there is an object ownership problem that's a little more subtle. Within main, the window object is created as a regular ...


5

After your comment, the only change i could say is to change private void jump(){ // called every frame if(jumping){ if(y >= -jumpHeight){ y -= jumpSpeed; }else{ jumping = false; falling = true; } } if(falling){ to private void jump(){ // called every frame if(jumping){ ...


5

createBlocks actually fills the blocks array, creation happens before that, so this name is a little bit misleading. I'd move the array creation inside the method and use it as a return value: private Block[][][] blocks; public Chunk(int chunkXNumber, int chunkZNumber) { .. blocks = createBlocks(chunkXNumber, chunkZNumber); } private Block[][][] ...


5

This code is good for tutorial use. Some suggestion: You are switching over 'x','y' and 'z' : function rotateCube( axis ) { switch ( axis ) { case 'x': mesh.rotation.x += 0.02; break; case 'y': mesh.rotation.y += 0.02; break; case 'z': mesh.rotation.z += 0.02; ...


5

Note: I'll probably be using some C++11 and C++14 features, so if you can't use such features, just ignore them. No raw new. material.AddTexture("grid_metal", new Texture("res/Metal4.jpg", GL_TEXTURE_2D, GL_LINEAR)); There is no need to use raw new here. Either refactor your Texture class to contain a pointer to some TextureImpl class, or use C++14 std::...


5

Resource management: I general, the code seems alright to me, but one thing you continue to do, if I recall from your previous questions, is to use raw pointers and manual resource/memory management. So I'd like to urge you to look into the standard smart pointers and start using them. The basic usage is: Resource is shared between instances of different ...


5

Well, your choice of backing-type for your collections of shaders, meshes and textures has a few interesting results: Your keys are pointers, not strings, and those pointers are compared/hashed. While the compiler may freely merge string-literals, it need not. Every self-respecting compiler in practice does it inside a single translation-unit, but outside ...


5

A few tips: One-stage initialization is preferred. Technically, everything has an initialization process, but by declaring that everywhere, you defeat the point of a constructor. It also aligns with the RAII principles where the object isn't even finished being created if any initialization fails. One of the exceptions here is that with rendering processes,...


Only top voted, non community-wiki answers of a minimum length are eligible