Improvement for on_entity_color_changed

Place to ask discuss and request the modding support of Factorio. Don't request mods here.
Post Reply
Pi-C
Smart Inserter
Smart Inserter
Posts: 1654
Joined: Sun Oct 14, 2018 8:13 am
Contact:

Improvement for on_entity_color_changed

Post by Pi-C »

TLDR
Whenever LuaEntity.color is set, defines.events.on_entity_color_changed will be raised, even if old and new color are the same. Please consider raising the event only if old and new color differ! Alternatively, it would also help if the old color was added to the event data so that mods can easily compare it with the new color.
What?
Start a new game and run the following commands from the console:

Code: Select all

/c script.on_event(defines.events.on_entity_color_changed, function(event) game.print(string.format("New color of %s: %s", event.entity.name.." ("..event.entity.unit_number..")", serpent.line(event.entity.color))) end)
/c p = game.player; v = p.surface.create_entity{name = "car", position = {p.position.x + 2, p.position.y}, force = p.force}
/c v.color = {r = 1}
/c v.color = {r = 1}
The game will show something like this:
on_entity_color_changed.png
on_entity_color_changed.png (228.29 KiB) Viewed 238 times
As you can see, the event has been raised both times I've assigned a value to v.color, although the color after the second change is the same one as the color after the first change.
Use case
My mods Autodrive and GCKI will make sure that the vehicles they control will always have the same color as the player who controls them (this could be somebody who is neither driver nor passenger). They keep track of the color of vehicles based on a prototype they support from the time the vehicle is created until it is mined or destroyed. If a player manually changes the color of a vehicle controlled by one or both mods (e.g. via the native GUI of spider-vehicles or with mods like Custom Color), my mods will update the stored color and reset it to that of the player controlling the vehicle. After the last mod has stopped to control a given vehicle, its color will be reset to the stored color.

If both mods are active, resetting vehicle colors involves running several checks and remote calls which cause some overhead that would be unnecessary if the event had been raised although old and new color are the same.
Not our business, the mods should handle this!
Working on it from both of my mods' ends, I've already invested a lot of time on making color changes and responses to color changes as efficient as possible. My optimizations so far:
  • If both mods are active, one of them ("server") will take care of color changes for the other ("client") as well. Only the server will listen to on_entity_color_changed and use remote calls to tell the client it should update its data.
  • Both mods will store and update the original color of any vehicle that is based on a prototype supported by one or both mods. In response to on_configuration_changed, the server will initiate synchronizing the list of original colors with the client.
  • Instead of using remote calls to ask the other mod for the player controlling a vehicle each time one of the mods wants to change colors (pull), they now inform each other per remote call when they start/stop controlling a vehicle (push). If a vehicle is controlled by both mods, the player controlling it on the other end will be stored with the vehicle data. Thus, when changing colors I rely on locally stored data whenever possible and only use remote calls if a vehicle is controlled by just one mod. (If one of the mods is removed, obsolete data will be deleted during cleanup.)
  • If both mods are active and the client wants to change a vehicle's color, it will just update its data and ask the server to change the color (server will skip the callback to client in this case).
  • The color of a vehicle will only be changed if old and new color are different.
  • Before a mod changes the color of a vehicle, it will store entity and expected new color. This way, it can skip the on_entity_color_changed event that was raised in response to its own color changes.
As on_entity_color_changed has been added quite recently (version 1.1.83, really working since 1.1.85), it is quite likely that a number of modders aren't aware that an event will be raised each time they set LuaEntity.color, so they do it without taking any precautions. If two or more mods compete to change the color of an entity (A changing to new color, B trying to reset it to old color while both colors are the same), this may result in a vicious circle leading to a stack overflow. Therefore, it's now important to check that old and new color differ before setting LuaEntity.color -- but comparing colors is not as trivial as

Code: Select all

util.table.compare(old_color, new_color)
because colors can be given in different formats (e.g. the above test would fail for {r=1} and {r=1, g=0, b=0, a=1} although both specify the same color). This is what I am using:

Code: Select all

require('util')

-- Map default Factorio player colors to color names
assert.colors = {
  default     = {r = 1.000, g = 0.630, b = 0.259},
  red         = {r = 1.000, g = 0.166, b = 0.141},
  green       = {r = 0.173, g = 0.824, b = 0.250},
  blue        = {r = 0.343, g = 0.683, b = 1.000},
  orange      = {r = 1.000, g = 0.630, b = 0.259},
  yellow      = {r = 1.000, g = 0.828, b = 0.231},
  pink        = {r = 1.000, g = 0.520, b = 0.633},
  purple      = {r = 0.821, g = 0.440, b = 0.998},
  white       = {r = 0.900, g = 0.900, b = 0.900},
  black       = {r = 0.500, g = 0.500, b = 0.500},
  gray        = {r = 0.700, g = 0.700, b = 0.700},
  brown       = {r = 0.757, g = 0.522, b = 0.371},
  cyan        = {r = 0.335, g = 0.918, b = 0.866},
  acid        = {r = 0.708, g = 0.996, b = 0.134},
  -- This is the default color for vehicles without an owner/locker in AD/GCKI!
  no_color    = {r = 0, g = 0, b = 0, a = 0.5}
}


-- Get color from name, hex, or table
assert.ascertain_color = function(color)

  assert.assert(color, {"table", "string", "nil"})

  local ret
  local c = type(color)

  -- Named colors or hex colors
  if c == "string" then
    ret = assert.colors[color] or
          (color:match("^#*[0-9a-fA-F]+$") and util.color(color))
    if ret then
      ret.a = 1
    end

  -- Lua color
  elseif c == "table" then
    ret = {
      r = color.r or color[1] or 0,
      g = color.g or color[2] or 0,
      b = color.b or color[3] or 0,
      a = color.a or color[4] or 1,
    }
  end

  return ret
end


-- Compare two colors. Returns true if the colors are the same
assert.compare_colors = function(c1, c2)

  c1 = assert.ascertain_color(c1)
  c2 = assert.ascertain_color(c2)

  return not (c1 or c2) or 
            ( c1 and c2 and util.table.compare(c1, c2) )             
end
As you can see, some overhead is necessary because you have to normalize the inputs before you can compare them. I guess it would be a lot cheaper if the game would take that load off modders' shoulders and only raise the event if old and new color really differ. If that's impossible/impractical for whatever reason, passing on the old color in the normalized form {r=…, g=…, b=…, a=…} with the event data would at least make it a bit cheaper for mods to compare the colors before they take any action.
A good mod deserves a good changelog. Here's a tutorial (WIP) about Factorio's way too strict changelog syntax!

Post Reply

Return to “Modding interface requests”