Code style guidelines

[Argmaster]

This issue is intended to aggregate discussions about code style practices in Cubyz codebase.

As of creation time of this issue, contribution guidelines state that:

Write correct and readable code

Explicitly handle all errors (except from those that can't happen)

Error handling usually means logging the error and continuing with a sensible default. For example if you can't read a file, then the best solution would be to write a message to std.log.err that states the file path and the error string. It is also fine to bubble up the error with try a few levels before handling it.

Not all errors can happen. Specifically in Cubyz the error.OutOfMemory cannot happen with the standard allocators (main.globalAllocator and main.stackAllocator). In this case catch unreachable is the right way to handle the error.

Choose the right allocator for the job

Cubyz has two main allocators.

• The main.stackAllocator is a thread-local allocator that is optimized for local allocations. Use for anything that you free at the end of the scope. An example use-case would be a local List.
• The main.globalAllocator is intended to be used for general purpose use cases that don't need to or can't be particularly optimized.

Sometimes it might also make sense to use an arena allocator utils.NeverFailingArenaAllocator, or a MemoryPool. But generally these should only be used where it makes sense.

Also it is important to note that Cubyz uses a different allocator interface utils.NeverFailingAllocator which cannot return error.OutOfMemory. Along with it come some data structures like main.List and more in utils which should be preferred over the standard library data structures because they simplify the code.

Free all resources

Everything you allocate, needs to be freed.
Everything you init needs to be deinited.
Everything you lock needs to be unlocked.
This needs to be true even for error paths such as introduced by try.
Usually you should be fine by using defer and errdefer. There is also leak-checking in debug mode, so make sure you test your feature in debug mode to check for leaks.

Follow the style guide

Most of the syntax is handled by a modified version of zig fmt and checked by the CI (see the formatting section above).

Other than there is not much more to it:

• Use camelCase for variables, constants and functions, CapitalCamelCase for types
• There is no line limit (I hate seeing code that gets wrapped over by 1 word, because of an arbitrary line limit), but of course try to be reasonable. If you need 200 characters, then you should probably consider splitting it or adding some well-named helper variables.
• Don't write comments, unless there is something really non-obvious going on.
• But in either case it's better to write readable code with descriptive names, instead of writing long comments, since comments will naturally degrade over time as the surrounding code changes.

Style practices to be discussed:

• Order of definitions: top-down vs bottom-up. Since zig does not carry legacy limitation from C language that definition must precede use, more natural, top-down declaration order is available. In top-down approach high level, more abstract (big picture) functions (and possibly other code objects) appear before lower level implementation details. Implementation details appear after they are used for the first time. Ordering usually do not apply to variables and fields, which are grouped separately. more more
• File as struct vs file as namespace for structs. Zig documentation states that Zig source files are implicitly structs, with a name equal to the file's basename with the extension truncated. @import returns the struct type corresponding to the file.. Therefore unless creating generic data structure, creation of simple struct can be tackled by placing fields directly in file. This has a side effect of promoting one struct per file and hence separation of functionality into dedicated files and removes redundant namespaces. On the other hand, this creates inconsistency with generic structs which have to be created as functions. Currently we can find all possible mixtures of structs, generics, enums and unions in codebase, hence making codebase inconsistent and making it hard to judge which notation to chose, since in many cases they are equivalent.
• @import. Zig provides @import builtin function which can be used to add files to build and access field and declarations inside them if declared public. Currently we are multiple different practices regarding importing elements from different modules. For example, importing src/blocks.zig is done sometimes by const blocks = @import("blocks.zig"); while in other cases const main = @import("root");const blocks = main.blocks;. There are technical reasons to use second form, but first once could be consistently replaced with second one in most cases.
• Import symbol aliasing. When using structs / enums / unions defined in different files, they are sometimes aliased, sometimes not. Although strict rules in that matter could harm readability, on the other hand not aliasing objects when they are used many times creates unnecessary code and additional tension in case of rename. Additionally, aliases sometimes are not used consistently, for example in src/items.zig, struct Block is aliased while imported const blocks = @import("blocks.zig"); const Block = blocks.Block; but then main.blocks.Block appears in code anyway. A rule could be to always alias if accessing object through two or more namespaces (main.blocks.) or when using object at least 3 times. This will also incentivize using more descriptive names to avoid collisions with module names.
• Location of fields when using file as struct. Fields are always public in zig, hence they are always part of struct interface and they are both definition and first use, being their own getters and setter. As a result they should be defined first in the struct.

[Argmaster]

My problem right now is that I wanted to work on extracting functions like registerTool, registerBlock etc from assets.zig, as mentioned in one of #1125 , but then I noticed that recipes are in items file, so I wanted to extract them, but I am not sure if I should follow Inventory design (file as struct) or blocks/items design.

[IntegratedQuantum]

Order of definitions: top-down vs bottom-up

Whether enforced by the language or not I prefer seeing the declaration of a thing before it's first usage.
This is enforced by the language for local variables, this is common practice for imports and struct field. So in my opinion it only makes sense to use the same pattern for helper functions too.

But in the end I think what matters most is that related functions are close to each other in the code.

File as struct vs file as namespace for structs.

Both have valid use cases. But most of the time I think declaring structs separately is the way to go, because it is better when having multiple structs in one file.
Promoting one struct per file is not something I want. I had that in Java and it made it just more difficult to navigate the code.

then I noticed that recipes are in items file, so I wanted to extract them

They are fine where they are in my opinion. Just like tools they conceptually belong to items.

import

Importing root is the preferred way, but it sometimes breaks ZLS, and until that's more stable I'd like to leave it for now.

Import symbol aliasing

Another tooling issue, until ZLS has a way to automatically create and destroy aliases it does not make sense to enforce it.

Location of fields when using file as struct

Fields should always be at the beginning of the structs, unless there is something that conceptually should be declared up top (e.g. import statement or sometimes other declarations)

[Argmaster]

They are fine where they are in my opinion. Just like tools they conceptually belong to items.

In Cubyz, files are not documented with top level comment. As a result, you don't have a clear statement describing what given file actually contains. You are saying that they conceptually belong to items, so please, make an thought experiment and try to formulate said top level comment, something along the lines of items.zig file contains....

---

Now when I try to do that, what I get is something along the lines of items.zig file contains logic of items, modifiers, tools and recipes. Each of those 4 interfaces is tied to each other, as they interact with each other, but at the same time each of those interfaces is also accessed separately form outside of items.zig file. (tool interface is accessed via registerTool function, recipe interface is accessed via registerRecipes etc.). As a result you have 4 interfaces which are technically separate things but they are in one namespace. As a result you have name collisions - registerRecipes and registerTools could be named just register if they were in separate files, same way blocks are separate from items. It would create this really desired repetability and consistency in design where by knowing high level design of one part you can easily understand design of other parts. I agree that items, tools, recipes and modifiers should be grouped, but in my opinion correct grouping would be to have them in separate files in one directory.

What we could do is to group them under assets folder with other assets:

• src/items.zig -> src/assets/items.zig
• src/items.zig -> src/assets/recipes.zig
• src/items.zig -> src/assets/tools.zig
• src/blocks.zig -> src/assets/blocks.zig
• src/models.zig -> src/assets/models.zig
• src/assets.zig -> src/assets/loader.zig

This means that each file has clearly stated purpose. Removes name collisions, as MaterialProperty can become just Property, getToolTypeByID can become getByID, toolTypeIterator can become just iterator etc. thanks to namespaces, it is now responsibility of user to properly alias those names or keep them in namespaces.

[IntegratedQuantum]

I will think about this.

[IntegratedQuantum]

In Cubyz, files are not documented with top level comment. As a result, you don't have a clear statement describing what given file actually contains.

There is a clear description: The file name.
I don't think top-level comments are a useful concept: They constantly need to be maintained (but their correctness cannot be enforced → more review work), they often contain redundant information, and they are are mostly ignored (at least I mostly ignore them when I work on other people's projects).

I agree with you that it would be nice if every file only had one nice and simple responsibility (like e.g. commands or materials), but in my opinion that's not always useful. The reason why it works so well for commands is, because each command has a clear purpose, and commands don't interact with each other, so if I implement a new feature for a command I really only need to touch that one file.

On the other hand Tools, BaseItems and Materials do interact quite a lot. And it's nice to work on them without having to switch between files all the time. And VSCode's minimap makes navigating even large files really nice in my opinion. Much nicer than using file tabs.
Furthermore I don't even see where you would put Materials in your proposal.

registerRecipes and registerTools could be named just register if they were in separate files, same way blocks are separate from items.
Removes name collisions, as MaterialProperty can become just Property

The same thing could be done with regular namespaces, it could easily be Material.Property.

toolTypeIterator can become just iterator

I think that most of the proposed name changes are neutral (it neither makes typing nor reading easier/harder), this one would even make things worse in my opinion, unless you want a separate file just for tool types.

What we could do is to group them under assets folder with other assets:

I don't think "assets" is a good way to group things. All of these things do completely different things.
An asset to me is just "something that gets loaded from a file". That really is not a useful way to order things.
Many things in the game are assets, but should we put half of the code into the src/assets folder?
What if we decide that loading tools from assets (a recent addition by the way) was a bad idea? Where do we put them instead?

[Argmaster]

There is a clear description: The file name. I don't think top-level comments are a useful concept: They constantly need to be maintained (but their correctness cannot be enforced → more review work), they often contain redundant information, and they are are mostly ignored (at least I mostly ignore them when I work on other people's projects).

Single word is not a description, it is a symbol - its open for interpretation.
The more words you use, the less room for interpretation there is. This is exactly why comments are either useless or easily get outdated - they either strictly describe some aspects of code and when this code changes they get outdated, or they are vague and provide no value. Technically file name is a comment too, just a very generic one.

I agree that comments are a burden and there is no room for them when project is in pre-first release stage. However, I do not agree that one should not pay attention to them, especially when they are done correctly, see numpy.fft or cPython json module. They exist to provide documentation of features in the module, a guide. This is very useful if module is stable, since it makes it so documentation is actually close to code compared to having separate markdown files.

I agree with you that it would be nice if every file only had one nice and simple responsibility (like e.g. commands or materials), but in my opinion that's not always useful. The reason why it works so well for commands is, because each command has a clear purpose, and commands don't interact with each other, so if I implement a new feature for a command I really only need to touch that one file.

Whether you will have to touch multiple files or not depends heavily on what you are doing. If you decide to change something in how commands work in general, by adding new field or whatever, you will have to touch command files too. It's only a matter of how hard requirements change. It is not possible to predict how requirements will change, so don't try too. Do what's best now, as often a best way to prepare for future complexity is to keep what here today as simple as possible.

Why don't we just place everything in one file (except of commands since they are so independent)? Maybe, just maybe because setting boundaries keeps complexity of dependencies manageable. If we keep stuff together and have direct access to everything, then everything is a dependency for us. Complexity grows exponentially.

On the other hand Tools, BaseItems and Materials do interact quite a lot. And it's nice to work on them without having to switch between files all the time. And VSCode's minimap makes navigating even large files really nice in my opinion. Much nicer than using file tabs.

I don't see a particular difference (between switching and scrolling). The problem is that the second you have to think where to look, do actual searching, you are wasting time. I'm a bit digressing here, but hear me out: If we follow a rule that implementation details are always below, then we can always scroll below current code an often we will find what we are looking for. This is a branch prediction hit, it is good for efficiency. The better our branch predictions are the, the less often we have go back to actually searching for things.

Regardless, if we are mentioning VSC features, you can also use Ctrl+P and write <file-name> to open a file. On top of that you can also do Ctrl+P and write @<symbol-name> to navigate to symbol. There is also Ctrl+P -> :<line-number to navigate to line and Ctrl+P -> #<symbol-name> too look for a symbol in all files, but the last one doesn't unfortunately work with zig :C

You neither need scroll nor tabs to navigate codebase.

Furthermore I don't even see where you would put Materials in your proposal.

This was not a detailed refactor proposal, more of a expression of a general idea. What I noticed is that you are placing a lot of functionality in one file without setting boundaries. I have seen that before at corporate scale, trust me, you would not want to navigate a 30+k lines of implementation Vulkan Buffer structure in C++. More stuff you have together the faster it will accumulate new changes growing out of control.

registerRecipes and registerTools could be named just register if they were in separate files, same way blocks are separate from items.
Removes name collisions, as MaterialProperty can become just Property

The same thing could be done with regular namespaces, it could easily be Material.Property.

toolTypeIterator can become just iterator

I think that most of the proposed name changes are neutral (it neither makes typing nor reading easier/harder),

This is not true. If it was true, namespaces would not overtake C style naming conventions in modern programming languages. If there is no name collision, renaming registerRecipes to register reduces length of the name by almost 50%. This means that user module that imports recipes_zig would now use recipes_zig.register instead of items_zig.registerRecipes. This means smaller diffs, shorter lines, less code. Applied consistently those gains add up. Shortening one name in one module used once somewhere else saves you 5 chars, but doing it for 100 functions used 500 times total saves u 2.5k chars, 250 words, literally an essay you will not have to read. You write the code once, you read it hundreds of times, make yourself read as little duplicated information as possible.

this one would even make things worse in my opinion, unless you want a separate file just for tool types.

Possible, naming things is hard.

What we could do is to group them under assets folder with other assets:

I don't think "assets" is a good way to group things. All of these things do completely different things. An asset to me is just "something that gets loaded from a file". That really is not a useful way to order things. Many things in the game are assets, but should we put half of the code into the src/assets folder? What if we decide that loading tools from assets (a recent addition by the way) was a bad idea? Where do we put them instead?

Naming things is hard, structuring is hard too. No matter what principles you chose, requirements may change. Does that mean we are not supposed to structure code at all? There is an antipattern describing this approach, it has been proven destructive.

I am not emotionally tied to grouping those files in a way I have shown, neither I care about the names, as long as the code is as efficient to traverse, read and change as possible.

[IntegratedQuantum]

Single word is not a description, it is a symbol - its open for interpretation.

It's just enough to tell me what it is about.

I do not agree that one should not pay attention to them, especially when they are done correctly, see numpy.fft or cPython json module

These are great example for comments that I would skip right over. "Just show me some code already"
I didn't think I'd ever say something like this, but honestly you'd probably be better of letting an AI write you gist, than relying on the poor skills of developers to keep their comments up to date.

This is very useful if module is stable, since it makes it so documentation is actually close to code compared to having separate markdown files.

Yes, I absolutely agree. However Cubyz source code is neither stable, nor intended to be used as a module in third-party projects.

Whether you will have to touch multiple files or not depends heavily on what you are doing.

Of course it does, but for commands in particular I'd only touch one file most of the time, in the rare case of a change in its interface I'd touch all files at once. In graph theory you'd call this a tree, if you change the node, you need to change the children, but there are no links between the children.

This is obviously different to the many dependencies inside the items.zig file.

Why don't we just place everything in one file

I don't think we need to talk about this. Let's try to avoid the straw man fallacy here.

What I noticed is that you are placing a lot of functionality in one file without setting boundaries.

I do set boundaries, and I do regular split files when they get too large, or collect too many different things.
To me personally 1000 lines of code is just simply the optimal file size when it comes to complicated concepts.
And of course I wouldn't want to see a 5000 line file anywhere in Cubyz.

renaming registerRecipes to register reduces length of the name by almost 50%.

Doesn't matter for functions. Most functions are called on a separate line, and whether the function name is 5 characters longer or not makes no difference to me.

literally an essay you will not have to read.

Those are not really words that you read. Reading a 250 word essay takes time and effort, reading 250 words of redundant variable names just get filtered out by my brain.

Does that mean we are not supposed to structure code at all?

Another straw man, I never said that, and the code in question isn't even unstructured to begin with in my opinion.

[Argmaster]

These are great example for comments that I would skip right over. "Just show me some code already"
I didn't think I'd ever say something like this, but honestly you'd probably be better of letting an AI write you gist, than relying on the poor skills of developers to keep their comments up to date.

This is "I came here to understand, own and (possibly) modify" mindset. You can't understand, own and modify everything, it's not a sustainable model. You can try going through numpy's code to understand how to use function X and all of the quirks of it's 6 parameters, but you will not capitalize on that knowledge since you had to only use it once. It would be much more efficient to read summary for that one argument you care about and forget.

I'll stop here, since we just clearly have different view on how code should be structured and that's okay. After all it is not my burden to maintain Cubyz, I can just read once and forget and do useful stuff anyway

I guess if I am wrong then it does not matter, and if I am right, you will maybe see for you self.

I think we should add what we established to the contribution guidelines so this it is clear that there is some opinion about that matter. This should be beneficial for future contributors to align better with your code style.

[IntegratedQuantum]

Please check if there is anything important that I forgot to mention.