[2.1] Unintuitive force behaviour on LuaRendering methods

[BraveCaperCat]

Quite strangely, the draw methods on LuaRendering will draw to all forces if the forces parameter is set to an empty table. Thankfully, this behaviour is documented, (otherwise this would definitely be going in Bug Reports) but I don't see any reason why anyone would want this behaviour or how it could arise without explicitly adding it in.

I'd like this confusing and unintuitive behaviour to be removed, or if not, an explanation of why it's there to begin with.

[Rseding91]

Why?

[robot256]

Quite strangely, the draw methods on LuaRendering will draw to all forces if the forces parameter is set to an empty table. Thankfully, this behaviour is documented, (otherwise this would definitely be going in Bug Reports) but I don't see any reason why anyone would want this behaviour or how it could arise without explicitly adding it in.

I'd like this confusing and unintuitive behaviour to be removed, or if not, an explanation of why it's there to begin with.

What should it do instead? Crash the game? Create invisible rendering objects?

[eugenekay]

From an API design perspective, leaving forces and players as optional parameters simplifies the function-calling stack in the trivial case of a Singleplayer game or Multiplayer-all-players-on-the-same-force (default), since you do not need to worry about them at all. If you are trying to render only to a specific player/force chances are good that you know what you are doing. Changing them to required parameters at this point would create Runtime errors for every mod which relies upon this default behaviour - leading to more Bug Reports.

[robot256]

From an API design perspective, leaving forces and players as optional parameters simplifies the function-calling stack in the trivial case of a Singleplayer game or Multiplayer-all-players-on-the-same-force (default), since you do not need to worry about them at all. If you are trying to render only to a specific player/force chances are good that you know what you are doing. Changing them to required parameters at this point would create Runtime errors for every mod which relies upon this default behaviour - leading to more Bug Reports.

I don't think OP minds that nil=all forces. I can speculate (based on my own similar experiences) they just got frustrated after spending a lot of time debugging some code that selectively removes forces from a list which appeared to be fine until it removed all the forces from the list and then seemed like the filter wasn't working at all.

The documentation is there, so the solution is trivial: "if table_size(force_list)>0 then draw(...)". If you want to argue that forces={} should crash, then you still need the if statement. I don't know what value a render object visible by no forces would have.

[eugenekay]

I don't see any reason why anyone would want this behaviour or how it could arise without explicitly adding it in.

I'd like this confusing and unintuitive behaviour to be removed, or if not, an explanation of why it's there to begin with.

I don't think OP minds that nil=all forces.

Null/nil generally means that no argument was provided to the function; an empty table would value-check to null since no forces match the selector, so the default behavior (all forces) makes total sense.

[BraveCaperCat]

Quite strangely, the draw methods on LuaRendering will draw to all forces if the forces parameter is set to an empty table. Thankfully, this behaviour is documented, (otherwise this would definitely be going in Bug Reports) but I don't see any reason why anyone would want this behaviour or how it could arise without explicitly adding it in.

I'd like this confusing and unintuitive behaviour to be removed, or if not, an explanation of why it's there to begin with.

What should it do instead? Crash the game? Create invisible rendering objects?

I'd expect it to create invisible rendering objects, but not doing anything at all would also be acceptable for me.

From an API design perspective, leaving forces and players as optional parameters simplifies the function-calling stack in the trivial case of a Singleplayer game or Multiplayer-all-players-on-the-same-force (default), since you do not need to worry about them at all. If you are trying to render only to a specific player/force chances are good that you know what you are doing. Changing them to required parameters at this point would create Runtime errors for every mod which relies upon this default behaviour - leading to more Bug Reports.

I don't understand what you're talking about. I'm not asking for forces to become a required parameter. I wasn't even talking about players at all. (although I should mention that it has the same unintuitive behaviour that forces does)

My problem is with how the methods behave when forces (or players) is an empty table.

From an API design perspective, leaving forces and players as optional parameters simplifies the function-calling stack in the trivial case of a Singleplayer game or Multiplayer-all-players-on-the-same-force (default), since you do not need to worry about them at all. If you are trying to render only to a specific player/force chances are good that you know what you are doing. Changing them to required parameters at this point would create Runtime errors for every mod which relies upon this default behaviour - leading to more Bug Reports.

I don't think OP minds that nil=all forces. I can speculate (based on my own similar experiences) they just got frustrated after spending a lot of time debugging some code that selectively removes forces from a list which appeared to be fine until it removed all the forces from the list and then seemed like the filter wasn't working at all.

The documentation is there, so the solution is trivial: "if table_size(force_list)>0 then draw(...)". If you want to argue that forces={} should crash, then you still need the if statement. I don't know what value a render object visible by no forces would have.

Yeah, I had absolutely no idea why a sprite only supposed to draw to forces that don't have nauvis unlocked (which was none of them) was drawing to all forces until I re-checked the documentation.

I don't see any reason why anyone would want this behaviour or how it could arise without explicitly adding it in.

I'd like this confusing and unintuitive behaviour to be removed, or if not, an explanation of why it's there to begin with.

I don't think OP minds that nil=all forces.

Null/nil generally means that no argument was provided to the function; an empty table would value-check to null since no forces match the selector, so the default behavior (all forces) makes total sense.

Even in lua, that wouldn't happen, since nil and {} are two distinct objects - they're not equal. I can't think it would be much different in the C++ code either.

Not to mention that you cannot iterate over a null or nil value.

[eugenekay]

From an API design perspective, leaving forces and players as optional parameters simplifies the function-calling stack in the trivial case of a Singleplayer game or Multiplayer-all-players-on-the-same-force (default), since you do not need to worry about them at all. If you are trying to render only to a specific player/force chances are good that you know what you are doing. Changing them to required parameters at this point would create Runtime errors for every mod which relies upon this default behaviour - leading to more Bug Reports.

I don't understand what you're talking about. I'm not asking for forces to become a required parameter. I wasn't even talking about players at all. (although I should mention that it has the same unintuitive behaviour that forces does)

My problem is with how the methods behave when forces (or players) is an empty table.

The function signatures for LuaRendering::draw_line() and friends all look like this:

Code: Select all

draw_line{color=…, width=…, gap_length?=…, dash_length?=…, dash_offset?=…, from=…, to=…, surface=…, time_to_live?=…, blink_interval?=…, forces?=…, players?=…, visible?=…, draw_on_ground?=…, only_in_alt_mode?=…, render_mode?=…}

This includes "forces?=..., players?=..."; both listed as optional(?); without it the default behaviour is provided. It can be reasonably assumed that a decent quantity of their usage is thus made without specifying a forces/players explicitly(I have not grep-counted the prevalence of individual functions in the Factorio mod corpus), so this behaviour is thus depended-upon. When omitted, the function is apparently passed a zero-length Array. The design of this syntax seems to be "limit to only these forces/players if I give you a list, otherwise show it to everybody".

If these functions' behaviour is changed such that omitting these parameters suddenly does not render anything, then Mods who use this function would have to change their calling parameters - it has become a de facto required parameter in order for the function to do anything.


I hope I explained what I was trying to convey! I am not a Factorio Staff/Developer; just somebody passionate about programming APIs. Thanks for reading.