Dev feature 3 + 6 - Mutable Templates & Non-Burnable or Non-transferable XOR condition #8

on-a-t-break · 2024-09-20T01:53:48Z

Adds Mutable Template Table with template_id key and schema_name + serialized_mutable_data fields
Adds createtempl2 for creating templates with explicit mutable data
Non-Burnable or Non-transferable XOR condition

jona-wilmsmann · 2024-10-12T12:37:49Z

include/atomicassets.hpp

@@ -370,6 +406,16 @@ CONTRACT atomicassets : public contract {
    config_t       config       = config_t(get_self(), get_self().value);
    tokenconfigs_t tokenconfigs = tokenconfigs_t(get_self(), get_self().value);

+    void create_template(
+        name & authorized_creator,


I am not a fan of passing names by reference, since they're essentially just 64 bit primitive types. Most likely compiles to exactly the same output anyways, so more of a nitpick than anything else.

Sure, can be set to pass by value. I always do everything by reference when I can if I know that the original isn't going to be mutated.

jona-wilmsmann · 2024-10-12T12:39:30Z

include/atomicassets.hpp

+        bool burnable,
+        uint32_t max_supply,
+        ATTRIBUTE_MAP & immutable_data,
+        ATTRIBUTE_MAP mutable_data = {}


is there a reason not to use a reference for the mutable data when you're using one for the immutable data? I also don't really like the default value, I'd rather see that explicitly when calling the function.

There's no reference for the mutable_data because the original createtempl action does not have a field for mutable_data (only immutable_data). It's only accessed by the new createtempl2 action.

Just looks a bit cleaner this way, as the function doesn't have to create an empty ATTRIBUTE_MAP & pass that over (if it was by reference)

jona-wilmsmann · 2024-10-12T12:40:40Z

include/atomicassets.hpp

+        uint64_t primary_key() const { return (uint64_t) template_id; }
+    };
+
+    typedef multi_index <name("templatedata"), template_data_s> template_data_t;


I think calling this templatedata could be confusing, because template data already exists as part of the normal template table. A name that makes it clear this is referring to mutable template data would be better imo.

I agree, wdy think about these:

templmutable

mutabletempl

jona-wilmsmann · 2024-10-12T12:41:34Z

include/atomicassets.hpp

@@ -370,6 +406,16 @@ CONTRACT atomicassets : public contract {
    config_t       config       = config_t(get_self(), get_self().value);
    tokenconfigs_t tokenconfigs = tokenconfigs_t(get_self(), get_self().value);

+    void create_template(


I would call this internal_create_template to be consistent with the other internal functions.

jona-wilmsmann · 2024-10-12T12:44:30Z

include/atomicassets.hpp

+    ACTION settempldata(
+        name authorized_editor,
+        name collection_name,
+        name schema_name,


I don't think schema_name should be a part of the action signature, as it can be inferred from the template_id. setassetdata also doesn't include the schema name

jona-wilmsmann · 2024-10-12T12:58:41Z

src/atomicassets.cpp

+        make_tuple(collection_name, schema_name, template_id, deserialized_old_data, new_mutable_data)
+    ).send();
+
+    // If entry doesn't exist, then emplace entry


This control flow has a problem where you could call it with empty new mutable data, and this empty data would then be emplaced. Doesn't break anything, but I think the following controlflow would be better and also clearer to understand (in pseudocode):

if no template data entry exists { check new_mutable_data must not be empty emplace } else { if new_mutable_data is empty { erase } else { modify } }

Added a condition for emplacing. It's a rather unique scenario & would likely only be executed by accident by users, but this just makes it a bit cleaner internally & more consistent with the entry modification functionality, while still not throwing an error to the user & breaking the transaction.

I'm leaning towards not having a check here to avoid breaking transactions when it's not critical / absolutely necessary.

if (template_data_itr == template_data.end() && new_mutable_data.size() > 0){ template_data.emplace(authorized_editor, [&](auto &_template_data) { _template_data.template_id = template_id; _template_data.schema_name = template_itr->schema_name; _template_data.mutable_serialized_data = serialize(new_mutable_data, schema_itr->format); }); }

jona-wilmsmann · 2024-10-12T13:01:41Z

src/atomicassets.cpp

+    int32_t template_id = current_config.template_counter++;
+    config.set(current_config, get_self());
+
+    if (transferable == false){


I'd prefer
check(burnable || transferable, "A template cannot be both non-transferable and non-burnable")

jona-wilmsmann · 2024-10-12T13:04:56Z

src/atomicassets.cpp

+            permission_level{get_self(), name("active")},
+            get_self(),
+            name("logsetdatatl"),
+            make_tuple(collection_name, schema_name, template_id, mutable_data, mutable_data)


old data in the log should be empty, not the mutable data as well.

… name

on-a-t-break · 2024-10-14T20:37:54Z

Incorporated all comments above, still need to decide the templatedata table name, otherwise ready for merging into CPU optimizations.

on-a-t-break added 6 commits September 6, 2024 15:43

dev-feature-3

6075419

dev-feeature-3-fixes

86b4708

dev-feature-3-fixes

4ff8b26

dev-feature-3 + dev-feature-6 merge

35dbb88

polymorph / overloading fix

d4857de

missing schema_name emplace on createtempl2

4d69ac0

jona-wilmsmann reviewed Oct 12, 2024

View reviewed changes

Fixes; *performed all fixes; *still need to decide templatedata table…

e7a3241

… name

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Dev feature 3 + 6 - Mutable Templates & Non-Burnable or Non-transferable XOR condition #8

Dev feature 3 + 6 - Mutable Templates & Non-Burnable or Non-transferable XOR condition #8

on-a-t-break commented Sep 20, 2024 •

edited

Loading

jona-wilmsmann Oct 12, 2024

on-a-t-break Oct 14, 2024

jona-wilmsmann Oct 12, 2024

on-a-t-break Oct 14, 2024 •

edited

Loading

jona-wilmsmann Oct 12, 2024

on-a-t-break Oct 14, 2024

jona-wilmsmann Oct 12, 2024

jona-wilmsmann Oct 12, 2024

jona-wilmsmann Oct 12, 2024

on-a-t-break Oct 14, 2024

jona-wilmsmann Oct 12, 2024

on-a-t-break Oct 14, 2024

jona-wilmsmann Oct 12, 2024

on-a-t-break commented Oct 14, 2024 •

edited

Loading

Dev feature 3 + 6 - Mutable Templates & Non-Burnable or Non-transferable XOR condition #8

Are you sure you want to change the base?

Dev feature 3 + 6 - Mutable Templates & Non-Burnable or Non-transferable XOR condition #8

Conversation

on-a-t-break commented Sep 20, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

on-a-t-break Oct 14, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

on-a-t-break commented Oct 14, 2024 • edited Loading

on-a-t-break commented Sep 20, 2024 •

edited

Loading

on-a-t-break Oct 14, 2024 •

edited

Loading

on-a-t-break commented Oct 14, 2024 •

edited

Loading