Opened 8 years ago

Last modified 5 years ago

#4076 new enhancement

[PATCH] Improve moddability of templates

Reported by: sanderd17 Owned by:
Priority: Should Have Milestone: Backlog
Component: Simulation Keywords: patch
Cc: Patch:

Description

Currently, when modders want to alter a certain kind of stat (like the cost), they have to alter a lot of templates. And the only way to alter the templates is to copy them to the mod and alter the value. It should be possible to only define the changes to the templates instead.

Currently, we have an inheritance chain where the child template references the parent.

<Entity parent="template_structure_defense_wall">

So plugging in a template in the inheritance chain isn't possible. As it would mean altering all child templates, and thus again copying those values.

It should be possible to inherit from a template with the same name, but in the lower-priority mod. That way a higher-priority mod can plug in a template that's referred to by the children, but not copy the values of the template itself.

Attachments (3)

extend_templates.diff (2.0 KB ) - added by sanderd17 8 years ago.
extend_templates.2.diff (2.4 KB ) - added by sanderd17 8 years ago.
Fixed a compilation issue, changed some comments and variable names
vfsExtendedTemplates.diff (18.2 KB ) - added by sanderd17 8 years ago.
New experiment, with altered VFS

Download all attachments as: .zip

Change History (16)

comment:1 by Karamel, 8 years ago

I have thought about the issue but can't come to an elegant feature that would do everything fine. The main reason is the virtual file system that loads only one version of a file with a given name, without opening it and changing that may add a lot of complexity for little purpose.

If we use an attribut in the root element for a template with the same name as the original, there cannot be multiple mods that alter the same template and the file loader must be modified to allow mounting multiple files with the same name (to keep track of the original template).

If we add a .EXTENDS (for example) to add a file that will alter the referenced template, there could still be only one mod that alter the original template (because of the file name). Using the filename for data also seems a bit wierd.

If we use random filename with an attribute in the root element, every file must be parsed before reading templates to know which template is altered by which file. This is an overkill and in most of the case useless. And we still don't know if there are multiple extensions which one has higher priority than the other. The mod priority is at file level only to check which one should be kept. The priority is not stored.

We can add an alteration list file which indicates which template is altered by which file and in which order (the file loader could make a special case of this file to track priority). There is no performance issue but from a moder point of view this file is redundant (it is only a technical workaround) and may easily be skipped, which leads to errors and headaches.

I remember having discussed long ago about inclusion from an xml file to an other (don't remember with whom, it may be elexis) which is very close to what is suggested. The conclusion was that there is already inheritance to include some parts of xml and it would lead to a real mess if there is an other way to do things that can do the same. Alterations should then be banned from the main mod anyway.

Considering this feature is required only for some mods that alter the main game, which will settle down one day, all these solutions may become pointless once the game is released. So appart from being not elegant enough, they would add a not that usefull feature to maintain. In a way, these mods are replacing the main mod, not extending it.

My suggestion is then to create an external tool like the template analyser that will generate the templates by alterating referenced ones. The alteration mods would then be sort of compiled from the public mod without touching anything from the engine. This still doesn't allow multiple mods from editing the same template but it could be spread among multiple files (like the sound or actor xml files).

"If there's no solution, there's no problem" :) (personal quote)

comment:2 by sanderd17, 8 years ago

Even if modifying the VFS is too hard, there are still possibilities with filenames.

Names like template.xml.EXTENDS.modname, or alterations to the directory, like putting the files in a templates_modname instead of regular templates.

Perhaps that thing with the directory is the easiest to do (seems quite trivial in TemplateLoader), though perhaps it's not that obvious for modders.

comment:3 by sanderd17, 8 years ago

Keywords: patch rfc added
Milestone: BacklogAlpha 21
Summary: Improve moddability of templates[PATCH] Improve moddability of templates

The above patch implements an extension like [templateName].EXTENDS.[modName].xml. I chose for this format for some reasons:

  • The .xml extension is still at the end, which means that file-extension bindings will still work on different systems, and the syntax highlighting on many text editors will also still be correct
  • The file resides in the same directory as other templates, which makes it easier to find out where a stat is coming from
  • The name includes a random modName part (which doesn't need to be the mod name per-se), to allow multiple mods to extend on the same template.

Apart from that, the code also forbids extensions to redefine parent templates, as I think allowing that would cause a mess. And by now explicitly erroring out on it, modders can't use it wrongly (f.e. by copy-pasting stuff) and cause problems later on.

The patch also includes an example that multiplies the max health of CCs with 10, this ofc shouldn't be committed.

by sanderd17, 8 years ago

Attachment: extend_templates.diff added

by sanderd17, 8 years ago

Attachment: extend_templates.2.diff added

Fixed a compilation issue, changed some comments and variable names

comment:4 by Karamel, 8 years ago

About the modName or whatever extension, wouldn't it be more natural for the VFS to automatically add them?

This avoids moders to add some random piece of cake in their filename and prevents having a single mod that extends multiple time the same template (which may lead to conflicts and so)

This is a matter of who has to write this marker, either the user or the loader. As it doesn't add anything for the user (allowing extending multiple times in one mod doesn't seem good to me), I'd say it's better to add it automatically (but this uglifies the populator).

As for priority if I read correctly the files are loaded in the order they are added, which is lower priority first. This is important to have always the same order (and the right one) not to get OOS in case of conflicts. I'm mostly thinking with various OS, I don't know how it works. If there can be multiple extensions of the same file in a mod, are they loaded in alphabetical order?

comment:5 by sanderd17, 8 years ago

The order is alphabetic, perhaps that's not the most wanted order, but it's certainly deterministic and won't cause an OOS.

About the VFS, yes, it would probably be better if the VFS handled this all. Especially, it would be great if the TemplateManager can ask from which mod a certain file comes, and if a mod with a lower priority than that also has that file. So you can still chain the extending templates.

But I don't really understand the VFS code well enough to do this, and it looks like it creates a final mod from the different loaded mods when the game is loaded. So it looks quite difficult to achieve this.

On the other hand, if we implement the patch I have now, and someone wants to fix the VFS later on to make this possible. Renaming the templates should be quite easy to script, so it wouldn't cause a huge issue for the modders (we can provide the script), and meanwhile they have something that easier to manage.

comment:6 by Karamel, 8 years ago

If we use the mod priority padded with 0000 as the modName, they will be loaded in the right order to be able to be chained. I don't really know C++ but I think it can be done in vfs_populate.cpp in AddFile, just rename the VfsPath to add m_realDirectory->Priority() when .EXTENDS.xml is found and it should do the trick

comment:7 by sanderd17, 8 years ago

That's a nice idea that could even work with the JS files if we choose the right filenames.

The code I have now should stay pretty much the same (as the templates still need to load multiple files), but the addition to the VFS code would make it possible to have shorter filenames indeed, and a less error-prone naming schema for the JS files.

by sanderd17, 8 years ago

Attachment: vfsExtendedTemplates.diff added

New experiment, with altered VFS

comment:8 by sanderd17, 8 years ago

The patch I uploaded now is not functional (though it does compile).

It's a work in progress where I altered the VFS to have access to files on lower-priority mods. And use that in the template loader.

Currently the Xeromyces and VFS part isn't finished yet, and the Xeromyces part will likey bump into some awkward XMB caching issues, as that's also indexed by the filename.

comment:9 by Itms, 8 years ago

Should this be pushed to A22?

comment:10 by elexis, 8 years ago

Milestone: Alpha 21Alpha 22

Feature freeze in 2 days.

comment:11 by wraitii, 7 years ago

Keywords: rfc removed

comment:12 by elexis, 7 years ago

Milestone: Alpha 22Backlog

Backlogging due to lack of progress

comment:13 by Imarok, 5 years ago

Component: UI & SimulationSimulation

Move tickets to Simulation as UI & Simulation got some sub components.

Note: See TracTickets for help on using tickets.