-
Notifications
You must be signed in to change notification settings - Fork 329
MegaMek Coding Style Guide
Consistency is an important factor in code readability. Any code submitted to MegaMek, MegaMekLab, or MekHQ should follow the coding style outlined here. Anything not covered in here can probably be left up to the coder's preference, but if there is any doubt the Google Java Style Guide is a good source.
Note that MegaMek has been around for decades and has had many contributors over the years. Not all of the current code base adheres to these guidelines. These are the guidelines for new code, and old code is being brought into compliance gradually. Note also that despite the general state of the code we are not looking for contributions that only mean to modernize the code without addressing a specific issue (in other words, refactors), especially as a first time contribution.
All source files must begin with a block comment that contains a copyright notice and the text of the license. Adjust the copyright year to be the current year when adding a new file. For existing files, this is to be merged at the top with existing copyrights that are NOT related to The MegaMek Team. If a new year and updating, hyphenate it with the current year. If it is already hyphenated, increment the end year to the current year.
In the below example, replace all comments of <Package Name> with either MegaMek, MegaMekLab, or MekHQ.
/*
* Copyright (C) 2025 The MegaMek Team. All Rights Reserved.
*
* This file is part of <Package Name>.
*
* <Package Name> is free software: you can redistribute it and/or modify
* it under the terms of the GNU General Public License (GPL),
* version 3 or (at your option) any later version,
* as published by the Free Software Foundation.
*
* <Package Name> is distributed in the hope that it will be useful,
* but WITHOUT ANY WARRANTY; without even the implied warranty
* of MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.
* See the GNU General Public License for more details.
*
* A copy of the GPL should have been included with this project;
* if not, see <https://www.gnu.org/licenses/>.
*
* NOTICE: The MegaMek organization is a non-profit group of volunteers
* creating free software for the BattleTech community.
*
* MechWarrior, BattleMech, `Mech and AeroTech are registered trademarks
* of The Topps Company, Inc. All Rights Reserved.
*
* Catalyst Game Labs and the Catalyst Game Labs logo are trademarks of
* InMediaRes Productions, LLC.
*
* MechWarrior Copyright Microsoft Corporation. <Package Name> was created under
* Microsoft's "Game Content Usage Rules"
* <https://www.xbox.com/en-US/developers/rules> and it is not endorsed by or
* affiliated with Microsoft.
*/Wildcard imports are permitted for more than 10 imports from the same package.
Braces follow the Kernigan and Ritchie style. The opening brace comes at the end of the line with the class declaration, method declaration, or control statement, separated from the closing round bracket by a space. The end brace is on a line by itself unless followed by a reserved word that is part of the block or connects a multi-block statement (else, catch, finally, while).
Examples:
class Foo {
// class definition
}
if (booleanExpr) {
// do a thing
} else {
// do a different thing
}
Braces are never omitted following if, else, for, while, or do, even for a single statement or an empty block:
if (booleanExpr) doSomething(); // wrong
if (booleanExpr) { //right
doSomething();
}
Empty blocks may appear on the same line if not part of a multi-block (if/else or try/catch) statement:
public void maybeDoTheThing() {} // ok
try {
defuseBomb();
} catch (WrongWireException ex) {} // not ok
All block indentations are four spaces. Never tabs. Each case block in a switch statement should be indented one level.
A space should be used within a line in the following circumstances:
- Between a reserved word and a parenthesis or a brace.
- Between a parenthesis and a brace.
- On both sides of a binary or ternary operator, with the exception of a method reference (::) or the dot separator (.).
- After a comma
When building boolean expressions using multiple operators, the component expressions should be grouped explicitly:
if ((a == b) && (c == d))
rather than
if (a == b && c == d)
If the number of extra parentheses makes the expression hard to read, consider introducing local boolean variables.
Any text strings displayed in the UI, either as UI elements or Report strings, should be stored in the appropriate messages.properties file to facilitate localization efforts.
Names of classes, interfaces, and enums should always be UpperCamelCase, with the first letter of every word capitalized and all other letters lowercase.
The names of methods and variables (including fields, local variables, and named parameters) should be lowerCamelCase, starting with a lower case letter and having the first letter of each additional word capitalized. The this. keyword as a prefix should only be used where necessary (e.g. in this.name = name).
The 'var' keyword can be used where it is helpful to avoid overly verbose declarations. It should however not be used when the type of the variable can't easily be seen from the declaration. In other words, avoid lines such as - var result = getResult();
Constants should be in all caps, with an underscore used to separate words. This includes fields that are declared static final and enum values.
We strive to use modern Java code. Still, it is important to keep in mind that the most concise code is not always the most readable. As many developers have to work with the code, it should prefer readability over being clever. Good ways to improve readability are:
- use descriptive names for variables and methods:
AmmoType ammoTypeinstead ofAmmoType at(write code for humans, not computers) - use intermediate boolean variables for complex tests, or
- factor out to methods. This can be very useful to hide streams
- add javadoc comments to methods and classes; write the javadoc for those who do not already know what the method does and do not want to read the method's code
Streams can be very useful and very concise. However, their readability is debatable and their preferred form comprises multiple lines. Therefore, preferrably
- streams should not be used directly within the test expression of an if () statement
- when applicable, stream results should be stored in variables with descriptive names or placed in methods with descriptive names
Optionals can in some places improve the code, especially when used as a method return value, by forcing the caller to deal with the possibility of an empty return value. However, in other places they can be more cluttersome than helpful. Therefore, they should be used, but judiciously.
All public or protected methods and classes should be documented with a JavaDoc comment, but is good practice to do so for package and private methods as well. Exceptions can be made when in trivial cases where it is obvious what the method does, such as basic setters and getters, but it is better to err on the side of over documenting. It is not necessary to repeat the documentation when overriding a method that is documented in the parent class unless the method is overridden to do something that is not covered in the documentation of the parent class.
When overriding an inherited method, the @Override annotation should always be used.
As a general rule, anything that is public is open to use, and once you publish a public API (or ABI) then its out there for the world to consume, other people may fork the project and use it for their own needs, so anything you delete may inadvertently break code on other peoples projects, so as a matter of courtesy we don't outright delete public methods, functions or classes, instead we annotate them with the Deprecated annotation.
@Deprecated(since="{SEMVER}", forRemoval=true)The since will receive the semantic version of the current nightly builds, so if you are deprecating a class or method and the current version of the nightly builds is going to be hard coded in there (EXAMPLE CODE WITH EXAMPLE VERSION: @Deprecated(since="0.50.06", forRemoval=true)). We always add the forRemoval=true to communicate to everyone that this method is going to be removed in a future release.
Every deprecated class or method is deleted in the next release cycle.
After you deprecate something you must make sure that it is not used anymore, you can't mark a piece of code as deprecated just because you dislike it, you need to offer an alternative to it, this alternative needs to serve the function the previous code was serving, and it must most of the time work as a in-place replacement, with little to no extra work required.
You can add the deprecated annotation to anything you want, variables, methods, enums, static functions, constructors, classes, interfaces, however only those that are not private are required to have the annotation. Private methods can only be used outside the class if you runtime editing of classes, javacode edits on classes at runtime or reflection access with access editing, and adding the annotation in there would not help anyone.
This is not a policy, but a suggestion how one of the developers personally uses the different log levels:
- Special rule: Don't add logs inside of loops/for-each, it can generate an obscene amount of log that generates barely any kind of value.
-
Formatting: Logger is static so it should be called
LOGGERin every place it's used. And when passing arguments to the log you should prefer to use the one which uses templated messages, e.g.:LOGGER.debug("Loaded {} with crew {}", entity, entity.getCrew());. - Fatal: The application died, this will dump EVERYTHING that is possible to dump of the context so it can be fixed IMMEDIATELY
- Error: Something unexpected happened, it is possible for it to happen but it should never happen, it has to be fixed and the error message should have enough information to make it possible to fix the problem, it is usually a blocker or will impact some functionality. Usually an error will be in places where you want to intercept an explicit exception like an IOError, etc, but maybe you dont have anyway of dealing with it because that is required to work otherwise the code can't function.
- Warning: Something wrong happened, maybe it could not load something it expected to be there. It's clearly problematic but it can be fixed and won't require engineering time to fix code, its usually wrong data or wrong config files, missing text on locales/i18n, etc. This should always contain the information necessary to guide the person to fix the error, like a wrongly configured planet file or academy file, an MTF or BLK file that has some error, a unit that doesnt pass validation, etc. Strive to make the warning have the information in there, so an experient user could fix the problem without outside help.
- Info: Simply put, its information, it will say what is happening at a speed that makes sense and in a frequency that won't necessarily cause problems to performance. It should be present in places that make sense in a "flow" to say what is happening, for example saying that you are saving, saying that the save was a success, saying that it is going to load the cache or clear the cache, things that usually are "big" and give an overall idea of what is happening and where, but it should not be at places that will create one entry per second, or hundreds of entries per second. Remember that the info log is logged in the release too, so we should not endup with anything that could potentially generate gigabytes of log for players.
- Debug: Debug logs will impact performance, it has the parameters that were used to call functions, it has the internal state of objects, it tells you what is going to happen and what happened, but you don't use logs inside loops, ever. If you need to workout what is happening in a loop you are entitled to make special considerations to it, like marking the number of iterations, give an aggregate information before the loop starts and another after it ends, add special handlers, etc.
- Trace: Use it to mark where the code is passing through, it will basically print the places the code or data is touching while the log level is trace. It should be used only as a last resort as it will punish you with the worst performance ever, and it is allowed to generate enormous amounts of logs. It should not contain anything that will take alot of time to run, no calls, not parsing data, traces are most successful when they only have simple messages with some string format arguments. It can be used inside loops and other places where logs are not advised.