Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Rework/save load system #209

Draft
wants to merge 14 commits into
base: main
Choose a base branch
from

Conversation

GalacticChimp
Copy link
Collaborator

@GalacticChimp GalacticChimp commented Nov 11, 2023

Goal

The primary goal is to simplify and stabilize the save system and to implement some additional improvements, as described in #171

Requires #237

Description

My main idea is to split (de)serialization logic to it's own objects, e.g. obj_ini is responsible for creating and loading it's own data via structs. This way, everything gets encapsulated and it becomes much clearer which variables/entities belongs to which object. Additionally, the structs are very easy to (de)serialize using GMLs out of the box tools/scripts. The new format is json.

Benefits

  • very easy to save-edit
  • creating serialization structs makes it really clear where each entity/variables is placed and how relationships between everything is structured, so this can be used as a foundation for refactoring the code into a more data-driven approach
  • might be able to vastly simplify Restart, serialize() can be simply called when restarting the game, which will make the whole process way easier and more robust
  • extension is very easy, just update serialize() and deserialize() of relevant object(s)
  • saving and loading should be more robust, i.e. less likely to crash, and if it crashes it should be obvious what went wrong.

Pitfalls

  • the uploaded example save file is 3.2MB, and it's still missing star data, there isn't really a good way to resolve this
    • UPDATE: the final save file is about 3.5MB. It's 3x bigger than ini format, but the tradeoff seems reasonable.

Additional info

I did some clean up and beautifications in save related files, but the actual changes are in:

  • Create_0 files - (de)serialization functions
  • save_load.Alarm_0 - handling save/load process
  • scr_load - TODO extracted deserialization logic to other objects
  • scr_save - extracted serialization logic to other objects
  • scr_squads - small json changes
  • scr_marine_struct - small json changes

Resolves #171

Attached example save file: save5.json

@GalacticChimp GalacticChimp added the I: Major Coding significant rework or refactor label Nov 11, 2023
@GalacticChimp GalacticChimp added this to the 0.8.3.0 milestone Nov 11, 2023
@GalacticChimp GalacticChimp self-assigned this Nov 11, 2023

}
controller = obj_controller.serialize();
ini = obj_ini.serialize();
Copy link
Collaborator Author

@GalacticChimp GalacticChimp Nov 11, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

it's bit hard to see in Github's PR, but the main idea is to simply call (de)serializers from relevant objects and script, and just save/load them as one big json.

@@ -1648,3 +1648,422 @@ if (vih>=5){
remov=string_length(string(temp[65])+string(temp[66])+string(temp[67])+string(temp[68])+string(temp[69]))+1;

action_set_alarm(2, 0);

#region serialization
Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

thinking about extracting this whole region into a separate file

popup_master_crafted,
select_wounded,
terra_direction,
stc: {
Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

as you can see, I reorganized variables a little, I wanted to see how a more data-driven approach would look like, and I like it so far

@@ -8,7 +8,7 @@
"option_windows_copy_exe_to_dest": false,
"option_windows_copyright_info": "",
"option_windows_description_info": "Become the 40,000th warhammer",
"option_windows_disable_sandbox": false,
"option_windows_disable_sandbox": true,
Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this is required to allow handling files in game executable folder


if (save_part=3){txt="Charting Sector";with(obj_controller){scr_save(2,obj_saveload.save_number);}trickle=10;save_part=4;}
if (save_part = 6) {
txt = "Praise to the Machine God";
Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'll re-add these last

@GalacticChimp
Copy link
Collaborator Author

GalacticChimp commented Nov 18, 2023

Serialization part is done, about 40% of the rework is done. Next step: saves.ini removal, then deserialization.

Also updated attached save file.

# Conflicts:
#	objects/obj_star/Create_0.gml
#	scripts/scr_load/scr_load.gml
Heretics,
Necrons = 13
Placeholder_1, // 12, not used anywhere currently, will be removed after creating proper structs for Factions
Necrons // 13
Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

small tweaks to eFACTION enum

}

function serialize_marine_struct(company, marine){
var copy_marine_struct = obj_ini.TTRPG[company, marine]; //grab marine structure
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
var copy_marine_struct = obj_ini.TTRPG[company, marine]; //grab marine structure
var copy_marine_struct = obj_ini.TTRPG[company][marine]; //grab marine structure

// General data and init

var _planetary_feature_data = {
_index: index,
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
_index: index,
index: index,

keys can't be variables unless variable_struct_set() is used so may as well just use index over _index

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

the point of the underscore, is to denote that this is a bookkeeping variable, which should not be touched during save-editing. I'll add comments to make this clearer. I did this way because I'm not sure if the order of elements in an array are preserved during jsonification and dejsonification.

chaos_corruption_level: chaos[company_index][entity_in_company_index],
experience: experience[company_index][entity_in_company_index],
age: age[company_index][entity_in_company_index],
is_special: spe[company_index][entity_in_company_index], // TODO Got no idea what this does
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this contains phychic power data, i'm currently trying to move all this into the struct then this can be deprecated

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ok, I'll remove this

entity_in_company_index = 1;

// HQ Marines??
for (entity_in_company_index = 1; entity_in_company_index <= 30; entity_in_company_index++) {
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this should be role data


// General data and init

var _planetary_feature_data = {
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

planetary features contain a much greater array of data and methods than are being serialised here, you may have catered for this already but just need to check

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

yes, this is covered in my local code, just didn't push everything yet

var _planet_data = {
_index: p,
//name: _planet, // TODO this is basically is_planet, does it make sense to store this?
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

not unless we start giving dedicated names/nicknames to planets

influence: p_influence[p],
raided: p_raided[p],
problems: [],
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
problems: [],
problems: [],
operatives:json_stringify(p_operatives[p])

p_operatives stores a struct with no methods so i presume stringifying it will be sufficient

@jhillacre
Copy link
Contributor

in #245, I propose removing was_zoomed from the save system. Once merged, you should be able to simplify your zoom obj now to a zoom bool.

@EttyKitty EttyKitty added PR: Major Coding Big enough to possibly clash with other PRs PR: Refactor Rewriting/restructuring code, while keeping general behaviour and removed I: Major Coding significant rework or refactor labels Dec 25, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
PR: Major Coding Big enough to possibly clash with other PRs PR: Refactor Rewriting/restructuring code, while keeping general behaviour
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Rework save system basic functionality
4 participants