Opened 6 years ago

Closed 5 years ago

Last modified 4 years ago

#2243 closed enhancement (fixed)

[PATCH] Allow actor changes on tech researches

Reported by: sanderd17 Owned by: sanderd17
Priority: Should Have Milestone: Alpha 16
Component: Core engine Keywords: patch
Cc: Patch:

Description

Sometimes it can be nice to allow actor changes on technologies. It is implemented as a normal technology where you can just add a

{"value": "VisualActor/Actor", "replace":"new_actor_template.xml"}

modification to the technology.

The patch here has an example template that changes every CC to a gymnasium model on town phase upgrade (this is only an example, not for inclusion in the svn). Note that it also works well with preview and construction entities.

Attachments (4)

actorTech.diff (6.1 KB) - added by sanderd17 6 years ago.
actorTech.2.diff (6.7 KB) - added by sanderd17 6 years ago.
actorTech.3.diff (9.1 KB) - added by sanderd17 6 years ago.
actorTech.4.diff (7.6 KB) - added by sanderd17 5 years ago.

Download all attachments as: .zip

Change History (20)

Changed 6 years ago by sanderd17

Attachment: actorTech.diff added

comment:1 Changed 6 years ago by historic_bruno

Some concerns:

  • The logic of the patch is confusing, wouldn't it be better to add a flag like m_IsFoundationActor to handle that special case, instead of relying on an arbitrarily empty string?
  • Why aren't foundation actors allowed to change?
  • How well does it work with actor hotloading? I think that was the issue I encountered when I tried implemented this tech, but it doesn't seem like it would be too difficult to solve.
  • Does it work with saved games?
Last edited 6 years ago by historic_bruno (previous) (diff)

comment:2 Changed 6 years ago by historic_bruno

Keywords: review removed

comment:3 Changed 6 years ago by sanderd17

Foundation actors aren't allowed to change as they use the same variable to hold the actor name, so when applying a tech mod, the foundation actor (the scaffolds) would turn into the finished building. Having a Boolean to represent wether this is a foundation actor or a regular one would fix this.

It shouldn't change hot loading capabilities, as it only caches the actor path, not the file. And it should work as well with saved games as the other c++ tech mods work with it.

Changed 6 years ago by sanderd17

Attachment: actorTech.2.diff added

comment:4 Changed 6 years ago by sanderd17

Added a boolean variable to check if it's a foundation or not.

And you're right it doesn't work with saved games. The problem is that the VisualActor? component is deserialised before the player components are deserialised. So it queries for a new actor on ownership change, but it gets the same actor returned as the modifications list is empty.

An additional problem is, because of this action, the property is cached in the technologyManager, so even new entities don't get the new actor.

Changed 6 years ago by sanderd17

Attachment: actorTech.3.diff added

comment:5 Changed 6 years ago by sanderd17

A new version, now the TechnologyManager? sends a message to every modified component after deserializing. So even components that are initialized before the technologyManager is get the message. It's an overload of messages, but this isn't too bad as it only happens once when you load a saved game.

I also added an other change that makes the ".xml" extension for actor names unnecessary (in other components, the ".xml" extensions were already unnecessary, or even forbidden). This allows you to have f.e. two actor files: building_I.xml and building_II.xml, and in the technology, you can just add "I" to the actor name to get the right name, as it will be added to the base name, and not after the extension.

comment:6 Changed 6 years ago by sanderd17

Keywords: review added

comment:7 Changed 6 years ago by leper

Milestone: Alpha 15Alpha 16

comment:8 Changed 6 years ago by wraitii

Some comments:
In CCmpVisualActor:
-I don't like "m_OriginalActorName", I'd prefer "m_BaseActorName", but that might be personal (mimics ObjectBase? vs ObjectEntry?, and it's quicker).
-Similarly, m_IsFoundationActor could be called m_IsFoundation.

I'm going to assume that since this just changes the actor, it doesn't change obstruction or anything so we'll need to rely on the tech creator being sensible.
My issue with this is that it's pretty annoying to do it for several items. If we replace civ centers, we need to be sure that we aren't replacing all civ centers. This could be changed to add some extension to the actor name, as you said, but it's still not really perfect.
Also this gives no way to replace specific actors: it doesn't seem like you can replace, say, the weapons of an entity. I don't really like that.

Maybe technologies aren't the proper place for this, or maybe we should add a completely new mode of differentiation just for those.

Last edited 6 years ago by wraitii (previous) (diff)

comment:9 Changed 6 years ago by sanderd17

The name changes look sensible.

For the other things, templates are completely moddable, so it also requires some sense of realism from the modder to apply the right footprint and obstruction. Requiring the same for actor changes isn't asking too much IMO.

As for replacing sub-actors (like weapons and stuff), IMO, we should first have a way to define some of those actors in the entity template. Afterwards, they can be made modifiable, just like the main actor. As normally, technologies only change things defined in the templates.

Changed 5 years ago by sanderd17

Attachment: actorTech.4.diff added

comment:10 Changed 5 years ago by sanderd17

Updated the patch. This time VisualActor? is caching the actor changes, so the TechnologyManager? doesn't have to send all those messages. Tested with saved games, and they work.

Looks ready for inclusion to me.

comment:11 Changed 5 years ago by sanderd17

Owner: set to sanderd17
Resolution: fixed
Status: newclosed

In 14928:

Make the actor tech-modifiable. Fixes #2243

comment:12 Changed 5 years ago by sanderd17

Keywords: review removed

comment:13 in reply to:  9 Changed 4 years ago by Nikos

Hi, sorry for the duplicate in #3283

Is this currently in the game? And more importantly, does it include individual prop changes yet (the most important time/file count saving part of it IMO)?

comment:14 Changed 4 years ago by sanderd17

This is in game, but does not include individual prop changes. Techs can only change entity templates, and entity templates can't choose props. So techs cannot alter props.

comment:15 Changed 4 years ago by Nikos

Is it considered low priority? Or not in to keep templates simpler?

I want to implement a fire-arrows tech, which should be a nice balancing addition for the core game as well, allowing civs with no ranged siege to be less disadvantaged in the aspect of ranged anti-structure/mechanical damage (maybe less vital there though due to capturing). Having two actors for each arrow firing unit and each of its ranks is too much work and extra files.

Other examples of commonly helpful functions would be having blacksmith upgrades partly change the visuals of units, adding new armor sets/shields/weapons etc or simulating bought/looted/equipped gear for an RPG game. Coupled with class-shared stat-changing techs (and a few enhancements to those) it could even replace the rank templates reducing the needed game files by a lot.

comment:16 Changed 4 years ago by sanderd17

It's not a priority now, because it's not important for the gameplay, and we have enough work. But if someone implements prop selection in the templates (and by extension in techs), that would be fine.

Note: See TracTickets for help on using tickets.