Hi! Just wanted to let you know that the mod accidentally creates new entities for the thermal solar panels from my mod Thermal Solar Power (Lite). They use the reactor prototype and won't be affected if the setting Heat pipe/reactor scaling is disabled. The Change mod entities setting makes no difference. You can read more in this thread.

The above creates a few issues: The new variants won't be recognized by my mod, so the script that generates heat can't be applied to them. And if the relevant setting is ever disabled, or the mod is uninstalled, the higher tier variants of the thermal panels will just disappear. I can create a compatibility fix for the first issue, but probably not for the second. EDIT: Another small issue: The replacement also happens without raising an event from the destroy() function, so my mod's storage table fills with non-existing entity IDs. Not sure yet whether I can fix that myself.

I figure it would be best if this mod avoids changing the thermal panels, which are outside of its intended scope.

Alright, so there are 4 separate issues:
a) "qualitized" versions of thermal solar panels not working
Fixing that would need specific logic on either end (either your mod looking for the quality variants, or my mod blacklisting these specific entities)
I would find it more elegant if your mod could handle the quality versions of the entities, but if you'd prefer otherwise, I'll think about how best to implement the blacklist: I could simply hardcode exceptions, but some interface for other mods to blacklist "technical" entities (ie. those requiring script support) would be more flexible. Giving the user the choice would in principle be desirable, but would add even more settings...
I guess either way, there should be some standardized way for how to deal with a mod making copies of script-dependent entities of another - either through a "my mod made a copy of this with that name - apply your script to that as well" interface (called from the modifier), or at least a "be careful with modifying and/or copying this entity" flag set by the entity creator.

b) entity loss on setting changes
Not trivial to mitigate, since you can't use migration scripts for setting changes. Keeping the internal entities for unmodified types would unnecessarily use up startup time and memory, but without those, my mod doesn't get any chance to react to the lost entities. I guess I could keep an internal table of all modified quality entities as well as previous startup settings, and on setting change go through the table and restore the normal quality machines, though that would still lose modules/items inside those machines, not to mention circuit connections, reactor temperature,...
Another option would be to add commands to revert categories to the base entities before switching off the startup setting - the same logic could then also be used to retroactively apply quality bonuses to existing entities, if desired, but communicating this process to the user might be non-trivial...

c) replacement events
I'm using the fast-replace option of create_entity, so eg. blueprint configuration automatically carries over - I'm setting raise_built = true, but apparently that doesn't fire a destroy event...
Do you know if that is the same behavior as for manual fast-replace? If so, your logic would need to factor in built events anyway (and including on_script_built in there shouldn't complicate that logic); otherwise, I should probably implement a custom event to let other mods know... Is there some standard for sending "my mod replaced your entity" events?

d) change mod entities inconsistent behavior
The "change mod entities" setting was mostly left over from earlier versions, since the way I implemented this for eg. trains didn't carry over as well for entity types with more individual entities. Currently, it affects only some categories (eg. trains, storage tanks, roboports), but not others (eg. belts, reactors, mining drills). I should probably either discontinue this option, replace it with a more granular "enforce whitelist per type" option or at least change the implementation to reference some compact export of vanilla/spage entity names, so it's consistent over all categories.

Thank you for the detailed response, I really appreciate it! I will go over it all carefully, do some additional testing and consider what specifically I want to request of you.

Quick question, does the event on_script_built really exist? I can't find it in the API. Maybe you are referring to script_raised_built, which I already use? (I account for 6 build and 7 destruction events.)

sorry, yes, you are right on that event name.

If fast replace in general doesn't send any destroy signal, then you would have to remove replaced entities from your storage table inside the corresponding on_built_entity/script_raised_built anyway. If it usually does send a signal, I could manually raise a custom event for you to react to.

All right. XD

Regarding manual fast-replacement: I did some testing with your mod disabled, and replacing any quality tier with another works without any issues. The old panel has its ID removed from the storage table, while the new panel has its ID added to it. With the technical variants from your mod enabled, replacement generally isn't possible at all, probably because they lack a shared fast_replaceable_group. Only the normal variant can be replaced. But even then, the script does not apply to the new technical variant.

I have already been working on a compatibility fix on my end: By checking for the presence of your mod, looking up all currently existing quality tiers in prototypes (including those added by other mods, excluding "normal" and "quality-unknown"), building a list of the names of all the technical variants through string concatenation and adding them to a reference list, the script at least applies to them. But the IDs of the entities replaced by your mod remain in my global table, which is not great. And the higher quality variants still disappear when the mod is uninstalled (or its relevant setting is disabled). I want to find a more reliable and generalized solution.

Anyway, still working on things, thinking things over.

Quick remark: It would certainly be helpful if the relevant destroy function would raise an event, such that other mods can know of its use on their entities by listening to script_raised_destroy. It can be written like .destroy({raise_destroy = true}). I am pretty sure this is also the convention: The responsibility is on the mod that destroys entities from other mods, to signal that it happened. For some reason, this requires explicit code.

I modified the code of your mod like that and found that it at least solves the issue with storage table bloat.

The destroy not raising an event is a legitimate oversight on my part. I expected qualitized entities to usually fast replace with their original, since they are just clones, and should inherit the fast replace category of the original.
Are you setting the fast replace category in data-final-fixes, or do the different panel tiers no longer fast replace with each other?

Well, my thermal panels don't actually have a fast_replaceable_group declared, because it's not needed. I only have one 3x3 and one 9x9 panel, and the game sorts out the replacement with different quality tiers on its own. But that changes with the "qualitized" variants. The base variant can be fast-replaced, but I don't exactly know why even that is possible, when any other combination/sequence doesn't work. They get a "Thermal solar panel is in the way" message, while original higher quality variants (placed before mod activation) won't get any feedback at all (just the same red color overlay).

I made a release with the raise_destroy bugfix, as well as an overhaul of the "ignore mod entities" functionality.

Concerning issue a), I added a mod data entry data.raw["mod-data"]["entity-clones"] that, in its data, contains a table mapping original entity names to lists of their clones.
In practice, you could simply read eg. data.raw["mod-data"]["entity-clones"].data["thermal-solar-panel-1"] for a list of the quality versions of it - and, should this catch on, maybe also eventually copies made by other mods.
I now also look at data.raw["mod-data"]["clone-blacklist"] before making my quality copies, so you could stop my mod affecting your entities in that way if necessary.

Unless there are some further issues, this should leave only issue b) open.

Now actually online, after I fixed some typos.

You move fast! I am still reading our discussion and mulling over what exactly I want to accomplish. XD

But this seems like a very fine solution (I did not know about "mod-data"!). This should allow me to immediately exclude just my thermal solar panels from being affected by your mod, while still giving me opportunity to create compatibility for it later.

I just downloaded the new version to try it out, but I get a crash with a message:

Failed to load mods: __more-quality-scaling__/data-final-fixes.lua:24: attempt to index field 'mod-data' (a nil value)
stack traceback:
    __more-quality-scaling__/data-final-fixes.lua:24: in main chunk
Mods to be disabled:
 - more-quality-scaling (1.4.8) [ ] Reset mod settings.

Interesting - maybe the mod-data section of data.raw isn't initialized if no other enabled mod adds any mod-data?
Should be fixed in 1.4.9.

It partially works now. But the game crashes when I try to write to the "clone-blacklist", and I can't make it work no matter what. Is it still WIP?

What is the crash message?

You probably need to initialize the category and create the mod-data entry (it probably doesn't exist when your mod runs):

if (not data.raw["mod-data"]) or (not data.raw["mod-data"]["clone-blacklist"]) then
    data:extend({{type="mod-data", name="entity-clones", data={}}})
end

and then add the entries to the data field, eg.:

data.raw["mod-data"]["clone-blacklist"].data["my_internal_entity"] = true

My blacklist logic looks like the following:

local cloneBlacklist = (data.raw["mod-data"]["clone-blacklist"] or {data={}}).data
if not cloneBlacklist[entityName] then doStuff() end

So the key needs to be the name of your entity, and the value needs to be truthy.

I haven't tested this functionality yet, but I can't see why it shouldn't work.

Concerning active compatibility, why would the simplest implementation be any more complicated than the following now?

local LIST_thermal_panels = {"tspl-thermal-solar-panel", "tspl-thermal-solar-panel-large"}
if data.raw["mod-data"] and data.raw["mod-data"]["entity-clones"] then
  for _,j in pairs(data.raw["mod-data"]["entity-clones"].data["tspl-thermal-solar-panel"] or {}) do
    table.insert(LIST_thermal_panels, j)
  end
  for _,j in pairs(data.raw["mod-data"]["entity-clones"].data["tspl-thermal-solar-panel-large"] or {}) do
    table.insert(LIST_thermal_panels, j)
  end
end

edit: added nil checks

Thank you. I did already try to initialize the "clone-blacklist" prototype myself, but even then, inserting the name in various ways (including the exact way you describe) didn't work. I also declared your mod an optional dependency in info.json and used the final prototype stage (data-final-fixes.lua).

When I just wrote:

data.raw["mod-data"]["clone-blacklist"].data["tspl-thermal-solar-panel"] = true

... I would get this error message on startup:

Failed to load mods: __thermal-solar-power-lite__/data-final-fixes.lua:19: attempt to index field 'clone-blacklist' (a nil value)
stack traceback:
    __thermal-solar-power-lite__/data-final-fixes.lua:19: in main chunk

But even when adding this before it:

if not mods["more-quality-scaling"] then return end

if (not data.raw["mod-data"]) or (not data.raw["mod-data"]["clone-blacklist"]) then
    data:extend({{type="mod-data", name="clone-blacklist", data={}}})
end

... the higher quality panels don't actually get excluded from having clones created of them. So I can't figure out what is going on. (As for active compatibility, I would like to test that later, since I prefer the first solution.)

I figured it out! I misunderstood the sequence in which things had to be done. Of course, my mod needs to write the names of the entities to the exclusion list BEFORE your mod references that list and decides whether to clone them. So I moved my code segment to the end of the first prototype stage, in data.lua, which is where I create all my prototypes.

Hi again. Just wanted to let you know that I have uploaded a new version of my mod! I skipped the stage of excluding my entities from being cloned, and instead managed to create proper compatibility for your mod! The heat generating script will now apply to the "qualitized" clones, and with correct output scaling as well. I also added a fast_replaceable_group to the thermal panel prototypes, just in case another mod clones them.

By the way, I think it would be smart to use more unique names for the "mod-data" prototypes, like "mqs-clone-blacklist". (EDIT: Never mind!) And then clearly denote in the code that it is best not changed, since other mods will depend on it. Finer exclusion criteria would not be bad either, so other mods can benefit from your mod, while avoiding the changes that cause them problems. (EDIT: Might be too much work, though.) But no need to implement such things for my sake! Just a few ideas for improvement.

The last real issue, I think, is the original b) issue of entity loss on setting changes or mod uninstallation. I don't have any ideas about that, so I will happily leave that to you to think about!

Sounds good!

I thought about finer exclusion criteria (something like a permission list, that details what other mods may do to your entities without requiring explicit compatibility code), but since my mod wouldn't really make use of them, I'm probably not in the best position to introduce such a system and incentivize others to use it. And the general "something-blacklist" idea might be taken up by others to grow into a more fine grained system anyway...

Maybe I should have named it "entity-clone-blacklist" to be more specific, but I don't think there are many compatibility issues with cloning items of fluids, so I think I'll keep it as is unless someone complains.

For issue b), the solution will probably be a set of commands to "un-qualitize" groups of entities before uninstallation/setting change, and maybe a popup message when someone loads a safe with missing entities to "load it again with the original setting, run the command, and then make the setting change" to keep the entities...
Restoring machines from a mod storage table would either need regular serialized backups of entity data (probably causing performance issues), or it would remove all configuration/modules/contents from the entities, which might be worse than straight up keeping them removed (at least that way it's easy to see the holes in the factory). I guess for some entity categories (eg. belts), the lost data might not be as critical - but an inconsistent implementation would probably cause more issue than it solved.

Yeah, it figures that finer exclusion criteria would be hard! It was also only after reading your answers a few times, that I understood the concept you were going for (a broadly shared, standardized permission list).

Regarding issue b): I am not a programmer, but I also get the impression that a set of commands would be needed, because the game doesn't seem to provide the necessary events (probably for performance reasons). Just in case it's ever needed, my own mod has a reset function that searches all surfaces for all thermal solar panels and rebuilds the storage table that is used by the script. Maybe that is what your mod needs, except, uh, more elaborate, and different. It seems that the (now obsolete) mod Cursed Filter Mining Drill by Ravran manages to do clone replacement back and forth without issue (modules and wire connections w. configuration remain), although it relies on an area selection tool.

I've been wondering if there is a simple way for missing entities to trigger a pop-up message in the console. The game clearly warns of missing prototypes, but maybe the mod still has to look for the missing prototypes itself on_configuration_changed with prototypes.get_entity_filtered({filter = "name", name = [EntityName]})? It's an interesting problem that I might explore a bit for myself.

I hope you find a solution you will be satisfied with, but don't overwork yourself!