PCK: Set VERSION_PATCH in header, factor out header magic

Unify pack file version and magic to avoid hardcoded literals.

`version.py` now always includes `patch` even for the first release in
a new stable branch (e.g. 3.2). The public name stays without the patch
number, but `Engine.get_version_info()` already included `patch == 0`,
and we can remove some extra handling of undefined `VERSION_PATCH` this
way.

Co-authored-by: Rémi Verschelde <rverschelde@gmail.com>

core/engine.cpp

@@ -94,11 +94,7 @@ Dictionary Engine::get_version_info() const {
 	Dictionary dict;
 	dict["major"] = VERSION_MAJOR;
 	dict["minor"] = VERSION_MINOR;
-#ifdef VERSION_PATCH
 	dict["patch"] = VERSION_PATCH;
-#else
-	dict["patch"] = 0;
-#endif
 	dict["hex"] = VERSION_HEX;
 	dict["status"] = VERSION_STATUS;
 	dict["build"] = VERSION_BUILD;

core/io/file_access_pack.cpp

@@ -34,8 +34,6 @@
 
 #include <stdio.h>
 
-#define PACK_VERSION 1
-
 Error PackedData::add_pack(const String &p_path, bool p_replace_files) {
 
 	for (int i = 0; i < sources.size(); i++) {
@@ -140,16 +138,14 @@ bool PackedSourcePCK::try_open_pack(const String &p_path, bool p_replace_files)
 	if (!f)
 		return false;
 
-	//printf("try open %ls!\n", p_path.c_str());
-
 	uint32_t magic = f->get_32();
 
-	if (magic != 0x43504447) {
+	if (magic != PACK_HEADER_MAGIC) {
 		//maybe at the end.... self contained exe
 		f->seek_end();
 		f->seek(f->get_position() - 4);
 		magic = f->get_32();
-		if (magic != 0x43504447) {
+		if (magic != PACK_HEADER_MAGIC) {
 
 			f->close();
 			memdelete(f);
@@ -161,7 +157,7 @@ bool PackedSourcePCK::try_open_pack(const String &p_path, bool p_replace_files)
 		f->seek(f->get_position() - ds - 8);
 
 		magic = f->get_32();
-		if (magic != 0x43504447) {
+		if (magic != PACK_HEADER_MAGIC) {
 
 			f->close();
 			memdelete(f);
@@ -172,9 +168,9 @@ bool PackedSourcePCK::try_open_pack(const String &p_path, bool p_replace_files)
 	uint32_t version = f->get_32();
 	uint32_t ver_major = f->get_32();
 	uint32_t ver_minor = f->get_32();
-	f->get_32(); // ver_rev
+	f->get_32(); // patch number, not used for validation.
 
-	if (version != PACK_VERSION) {
+	if (version != PACK_FORMAT_VERSION) {
 		f->close();
 		memdelete(f);
 		ERR_FAIL_V_MSG(false, "Pack version unsupported: " + itos(version) + ".");

core/io/file_access_pack.h

@@ -37,6 +37,11 @@
 #include "core/os/file_access.h"
 #include "core/print_string.h"
 
+// Godot's packed file magic header ("GDPC" in ASCII).
+#define PACK_HEADER_MAGIC 0x43504447
+// The current packed file format version number.
+#define PACK_FORMAT_VERSION 1
+
 class PackSource;
 
 class PackedData {

core/io/pck_packer.cpp

@@ -30,6 +30,7 @@
 
 #include "pck_packer.h"
 
+#include "core/io/file_access_pack.h" // PACK_HEADER_MAGIC, PACK_FORMAT_VERSION
 #include "core/os/file_access.h"
 #include "core/version.h"
 
@@ -68,11 +69,11 @@ Error PCKPacker::pck_start(const String &p_file, int p_alignment) {
 
 	alignment = p_alignment;
 
-	file->store_32(0x43504447); // MAGIC
-	file->store_32(1); // # version
-	file->store_32(VERSION_MAJOR); // # major
-	file->store_32(VERSION_MINOR); // # minor
-	file->store_32(0); // # revision
+	file->store_32(PACK_HEADER_MAGIC);
+	file->store_32(PACK_FORMAT_VERSION);
+	file->store_32(VERSION_MAJOR);
+	file->store_32(VERSION_MINOR);
+	file->store_32(VERSION_PATCH);
 
 	for (int i = 0; i < 16; i++) {
 

core/version.h

@@ -28,6 +28,9 @@
 /* SOFTWARE OR THE USE OR OTHER DEALINGS IN THE SOFTWARE.                */
 /*************************************************************************/
 
+#ifndef GODOT_VERSION_H
+#define GODOT_VERSION_H
+
 #include "core/version_generated.gen.h"
 
 // Godot versions are of the form <major>.<minor> for the initial release,
@@ -38,18 +41,18 @@
 // forward-compatible.
 // Example: "3.1"
 #define VERSION_BRANCH "" _MKSTR(VERSION_MAJOR) "." _MKSTR(VERSION_MINOR)
-#ifdef VERSION_PATCH
+#if VERSION_PATCH
 // Example: "3.1.4"
 #define VERSION_NUMBER "" VERSION_BRANCH "." _MKSTR(VERSION_PATCH)
+#else // patch is 0, we don't include it in the "pretty" version number.
+// Example: "3.1" instead of "3.1.0"
+#define VERSION_NUMBER "" VERSION_BRANCH
+#endif // VERSION_PATCH
+
 // Version number encoded as hexadecimal int with one byte for each number,
 // for easy comparison from code.
 // Example: 3.1.4 will be 0x030104, making comparison easy from script.
 #define VERSION_HEX 0x10000 * VERSION_MAJOR + 0x100 * VERSION_MINOR + VERSION_PATCH
-#else
-// Example: "3.1"
-#define VERSION_NUMBER "" VERSION_BRANCH
-#define VERSION_HEX 0x10000 * VERSION_MAJOR + 0x100 * VERSION_MINOR
-#endif // VERSION_PATCH
 
 // Describes the full configuration of that Godot version, including the version number,
 // the status (beta, stable, etc.) and potential module-specific features (e.g. mono).
@@ -64,3 +67,5 @@
 // Same as above, but prepended with Godot's name and a cosmetic "v" for "version".
 // Example: "Godot v3.1.4.stable.official.mono"
 #define VERSION_FULL_NAME "" VERSION_NAME " v" VERSION_FULL_BUILD
+
+#endif // GODOT_VERSION_H

editor/editor_export.cpp

@@ -32,6 +32,7 @@
 
 #include "core/crypto/crypto_core.h"
 #include "core/io/config_file.h"
+#include "core/io/file_access_pack.h" // PACK_HEADER_MAGIC, PACK_FORMAT_VERSION
 #include "core/io/resource_loader.h"
 #include "core/io/resource_saver.h"
 #include "core/io/zip_io.h"
@@ -970,11 +971,12 @@ Error EditorExportPlatform::save_pack(const Ref<EditorExportPreset> &p_preset, c
 
 	int64_t pck_start_pos = f->get_position();
 
-	f->store_32(0x43504447); //GDPC
-	f->store_32(1); //pack version
+	f->store_32(PACK_HEADER_MAGIC);
+	f->store_32(PACK_FORMAT_VERSION);
 	f->store_32(VERSION_MAJOR);
 	f->store_32(VERSION_MINOR);
-	f->store_32(0); //hmph
+	f->store_32(VERSION_PATCH);
+
 	for (int i = 0; i < 16; i++) {
 		//reserved
 		f->store_32(0);
@@ -1049,7 +1051,7 @@ Error EditorExportPlatform::save_pack(const Ref<EditorExportPreset> &p_preset, c
 
 		int64_t pck_size = f->get_position() - pck_start_pos;
 		f->store_64(pck_size);
-		f->store_32(0x43504447); //GDPC
+		f->store_32(PACK_HEADER_MAGIC);
 
 		if (r_embedded_size) {
 			*r_embedded_size = f->get_position() - embed_pos;

methods.py

@@ -67,8 +67,7 @@ def update_version(module_version_string=""):
     f.write("#define VERSION_NAME \"" + str(version.name) + "\"\n")
     f.write("#define VERSION_MAJOR " + str(version.major) + "\n")
     f.write("#define VERSION_MINOR " + str(version.minor) + "\n")
-    if hasattr(version, 'patch'):
-        f.write("#define VERSION_PATCH " + str(version.patch) + "\n")
+    f.write("#define VERSION_PATCH " + str(version.patch) + "\n")
     f.write("#define VERSION_STATUS \"" + str(version.status) + "\"\n")
     f.write("#define VERSION_BUILD \"" + str(build_name) + "\"\n")
     f.write("#define VERSION_MODULE_CONFIG \"" + str(version.module_config) + module_version_string + "\"\n")

platform/windows/godot_res.rc

@@ -4,10 +4,6 @@
 #define _MKSTR(m_x) _STR(m_x)
 #endif
 
-#ifndef VERSION_PATCH
-#define VERSION_PATCH 0
-#endif
-
 GODOT_ICON ICON platform/windows/godot.ico
 
 1 VERSIONINFO

scene/resources/packed_scene.cpp

@@ -39,7 +39,7 @@
 #include "scene/gui/control.h"
 #include "scene/main/instance_placeholder.h"
 
-#define PACK_VERSION 2
+#define PACKED_SCENE_VERSION 2
 
 bool SceneState::can_instance() const {
 
@@ -1095,7 +1095,7 @@ void SceneState::set_bundled_scene(const Dictionary &p_dictionary) {
 	if (p_dictionary.has("version"))
 		version = p_dictionary["version"];
 
-	ERR_FAIL_COND_MSG(version > PACK_VERSION, "Save format version too new.");
+	ERR_FAIL_COND_MSG(version > PACKED_SCENE_VERSION, "Save format version too new.");
 
 	PoolVector<String> snames = p_dictionary["names"];
 	if (snames.size()) {
@@ -1283,9 +1283,7 @@ Dictionary SceneState::get_bundled_scene() const {
 		d["base_scene"] = base_scene_idx;
 	}
 
-	d["version"] = PACK_VERSION;
-
-	//d["path"]=path;
+	d["version"] = PACKED_SCENE_VERSION;
 
 	return d;
 }

version.py

@@ -2,6 +2,7 @@ short_name = "godot"
 name = "Godot Engine"
 major = 3
 minor = 2
+patch = 0
 status = "beta"
 module_config = ""
 year = 2020