Dependencies: Are "optional requirements" possible at all?

[eradicator]

I made it global so I could use player.print in the external function that prints the nag message. Now I do it this way:

You should never *ever* do that. If you need something in a function, pass it as a parameter to the function. That's what parameters are for. And as a bonus it's also cheaper than reading + writing to a global value. (When i say "cheaper" i mean it's a measurable difference. Even if that difference is very very small indeed.)

By the way, what is cheaper: Passing on the whole player (complete local table) and using player.print in the other function, or reading/passing on the player.player_index and using game.get_player(index).print? My gut feeling tells me I should've used the index … :-)

All LuaSomething objects are actually wrapper tables that the game constructs on the fly. Constructing them isn't free, albeit obviously also not expensive. But whenever you have a reference ("table") to a LuaSomething (LuaPlayer in this case) you should try to pass it on if it's not super-difficult to do so.

game.connected_players doesn't seem to exist in singleplayer mode.

When I test it in singleplayer it works and returns a table with one player, myself.

+1. Works fine "on my machine".

If you don't need the player for anything else you can shorten it to:

I need to check whether the player is admin, and I need to pass on the table to the function; I think I read somewhere that you should make a local copy of a table if you access it more than once.

If you *do* need the player for something else then obviously you can't shorten it :). In that case the local reference is the right way to do it. And as eduran already explained, there's a big difference between "reference" and "copy". References to the same object will change the object.

Code: Select all

local x = {a = 1}
local b = x
b.a = 2
game.print(x.a) --prints "2"

There's a function called util.table.deepcopy() that will make an actual copy of a *native* lua table. LuaSomething objects however are just wrappers and copying them will do nothing.

[Pi-C]

game.connected_players doesn't seem to exist in singleplayer mode.

When I test it in singleplayer it works and returns a table with one player, myself.

So stupid! Reading --> understanding --> doing other stuff --> forgetting: That's how it goes with me all the time … The construct didn't work in on_configuration_changed, but you already explained that:

Players join afterwards and get a savegame in which on_configuration_changed already ran

Testing for is_multiplayer worked for me because it hid the part where I looked for admins among game.connected_players and simply printed to game.get_player(1) instead. Combine that with some misguided preconceptions on attribute name (like, I checked for game.is_multiplayer, so I certainly should check for player.is_admin) and there is a nice trail of red herrings to follow, interpret stuff wrongly, and introduce new errors .

[eradicator]

game.connected_players can be an empty table {} but not nil.

game.get_player(1) instead.

If you want to start with good habits don't ever ever use numeric indexes like that. Players are not guaranteed to exist. They could not have joined yet (headless start) or been deleted (on_player_removed).

Finally it's player.admin, not player.is_admin :p.

[Pi-C]

game.connected_players can be an empty table {} but not nil.

Strange, can't reproduce that anymore. Anyway, wouldn't trying to access an element of an empty table (e.g. extract player from game.connected_players and check player.admin) give some error?

game.get_player(1) instead.

If you want to start with good habits don't ever ever use numeric indexes like that. Players are not guaranteed to exist. They could not have joined yet (headless start) or been deleted (on_player_removed).

I believed that index 1 would be quite safe to use for singleplayer mode, but it seems I'd better avoid it altogether then.

Finally it's player.admin, not player.is_admin :p.

I know. Still it's easy to mix these up if checking different properties where some are named "is_SOMETHING" and others just "SOMETHING". When coding, I don't think "X should happen if the player has the flag 'admin' set", but "X should happen if the player is an admin". From there it's just a small step to write is_admin -- and then things break and even if the error message showed that is_admin doesn't exist I'd rather think "Where did I unset it?" than "Oh, I used a wrong property name!" because it looks right (i.e. corresponds to my expectations). I figured it out in the end, but with so many different names for classes, events, tables, properties etc., it's impossible to memorize them all and I regularly (not only in the context of Factorio obviously, as this is my first attempt at modding) waste a lot of time because of such mistakes.

[Qon]

game.connected_players can be an empty table {} but not nil.

Strange, can't reproduce that anymore. Anyway, wouldn't trying to access an element of an empty table (e.g. extract player from game.connected_players and check player.admin) give some error?

Yes, which is why you shouldn't do that. Same thing as the numeric index thing. But you can do it if you check the length first. But the sane thing to do is to do a proper loop.

[eduran]

Strange, can't reproduce that anymore. Anyway, wouldn't trying to access an element of an empty table (e.g. extract player from game.connected_players and check player.admin) give some error?

Depends on how you access the element.

Code: Select all

local empty_table = {}

-- not a problem, because loop body does not even run:
for k, v in pairs(empty_table) do
  game.print(v.name)
end

local player_1 = empty_table[1]  -- also OK, but player_1 == nil
game.print(player_1.name)  -- now you get a trying-to-index-nil error

-- to avoid that error:
if player_1 then
  game.print(player_1.name)
end

[eradicator]

game.connected_players can be an empty table {} but not nil.

Strange, can't reproduce that anymore. Anyway, wouldn't trying to access an element of an empty table (e.g. extract player from game.connected_players and check player.admin) give some error?

Short answer: No. Long answer: Depends on *how* you're doing it. In Lua any index/key is mapped to "nil" unless a value is assigned. Thus in an empty table all keys return nil. (Edit: Meh, had to go afk before i could post and in the mean time @eduran sneakily stole the answer. Posting anyway. More examples are always good :p)

Code: Select all

local x = {}
if x[5] == nil then
  game.print('say "not found" once')
  end
for k,v in pairs(x) do
  game.print('this will never print')
  end
if x[5].admin then --"error, attempt to index a nil value"
 end

I know. Still it's easy to mix these up if checking different properties where some are named "is_SOMETHING" and others just "SOMETHING".[...] it's impossible to memorize them all

Memorization gets better with time. As a generic rule-of-thumb everything that's named "is_what" is a read-only property, and most of the time these are functions. Then you can remember: ".admin can be written so it can't be named is_admin".

[Pi-C]

game.connected_players can be an empty table {} but not nil.

Strange, can't reproduce that anymore. Anyway, wouldn't trying to access an element of an empty table (e.g. extract player from game.connected_players and check player.admin) give some error?

Yes, which is why you shouldn't do that. Same thing as the numeric index thing. But you can do it if you check the length first. But the sane thing to do is to do a proper loop.

Speaking of which: I've sometimes seen loops such as

Code: Select all

for _, x in pairs() …

I assume "_" was used because you technically need 2 variables but only really care for the second value. In other words, "_" would not be a special variable name but rather a conventional name agreed upon to mean "ignore/discard this value". Is that correct?

[eduran]

Speaking of which: I've sometimes seen loops such as

Code: Select all

for _, x in pairs() …

I assume "_" was used because you technically need 2 variables but only really care for the second value. In other words, "_" would not be a special variable name but rather a conventional name agreed upon to mean "ignore/discard this value". Is that correct?

Correct. '_' is a valid variable name and you could access the key inside the loop body by using it, but it is usually used to indicate that you do not care about the assigned value.

(Edit: Meh, had to go afk before i could post and in the mean time @eduran sneakily stole the answer. Posting anyway. More examples are always good :p)

You have no idea how often that happens to me I have a tendency to get sidetracked while answering and when I remember it half an hour later someone else has beaten me to the punch.

[eradicator]

Speaking of which: I've sometimes seen loops such as

Code: Select all

for _, x in pairs() …

I assume "_" was used because you technically need 2 variables but only really care for the second value. In other words, "_" would not be a special variable name but rather a conventional name agreed upon to mean "ignore/discard this value". Is that correct?

Correct. '_' is a valid variable name and you could access the key inside the loop body by using it, but it is usually used to indicate that you do not care about the assigned value.

Sometimes ppl then go ahead and actually use the value of _. I do not have positive feelings towards these people :p. Btw, if you actually only care for the key you can technically do

Code: Select all

for key in pairs() do

and the returned value will simply be discarted. But personally i prefer to always write it out so it's clear that i actually want the key and didn't forget something:

Code: Select all

for key,_ in pairs() do

It's all about readability. "Why waste time on thinking of a name for something that you'll never call by name?"

[Pi-C]

Speaking of which: I've sometimes seen loops such as

Code: Select all

for _, x in pairs() …

I assume "_" was used because you technically need 2 variables but only really care for the second value. In other words, "_" would not be a special variable name but rather a conventional name agreed upon to mean "ignore/discard this value". Is that correct?

Correct. '_' is a valid variable name and you could access the key inside the loop body by using it, but it is usually used to indicate that you do not care about the assigned value.

OK.

Btw, if you actually only care for the key you can technically do

Code: Select all

for key in pairs() do

and the returned value will simply be discarted. But personally i prefer to always write it out so it's clear that i actually want the key and didn't forget something:

Code: Select all

for key,_ in pairs() do

It's all about readability. "Why waste time on thinking of a name for something that you'll never call by name?"

Yes, better write a bit more than making lean code that you'll have trouble understanding a week later.

Getting off-topic now: One of the mods creates a new item, using deepcopy to copy the recipe from the vanilla item. I want to read the crafting time (energy_required) for that new item, but If I try to access data.raw.recipe['item'].energy_required from data-updates.lua, I get a nil value. However, energy_required has a default value, and that value is also used in the game. I thought I could use the value from the vanilla item, but it isn't available there either. Guess it's inherited from the prototype later on?

My idea was to make a local table with table.deepcopy(data.raw.recipe['vanilla-item']) and use reload() on it. But how do I use that function? This doesn't work:

Code: Select all

local test = table.deepcopy(data.raw.recipe['vanilla-item'].reload())

It results in the error "attempt to call field 'reload' (a nil value)". How do I do it right?

[eduran]

I want to read the crafting time (energy_required) for that new item, but If I try to access data.raw.recipe['item'].energy_required from data-updates.lua, I get a nil value. However, energy_required has a default value, and that value is also used in the game. I thought I could use the value from the vanilla item, but it isn't available there either. Guess it's inherited from the prototype later on?

You are looking at the prototype, there is nothing to inherit from. The default value is used whenever a property is not defined. If you read energy_required and get nil, you know it is using the default value.

My idea was to make a local table with table.deepcopy(data.raw.recipe['vanilla-item']) and use reload() on it. But how do I use that function? This doesn't work:

Everything that is documented in https://lua-api.factorio.com/latest/index.html is meant for control.lua scripting, not the data stage. The reload() method is used to reload the recipe when the prototype changes during a game (e.g. because a mod was added or a startup setting changed). What are you trying to achieve by calling it?

[Pi-C]

I want to read the crafting time (energy_required) for that new item, but If I try to access data.raw.recipe['item'].energy_required from data-updates.lua, I get a nil value. However, energy_required has a default value, and that value is also used in the game. I thought I could use the value from the vanilla item, but it isn't available there either. Guess it's inherited from the prototype later on?

You are looking at the prototype, there is nothing to inherit from. The default value is used whenever a property is not defined. If you read energy_required and get nil, you know it is using the default value.

I'll know it's using the default value, but I won't know what that value will actually be (unless I look in the documentation).

My idea was to make a local table with table.deepcopy(data.raw.recipe['vanilla-item']) and use reload() on it. But how do I use that function? This doesn't work:

Everything that is documented in https://lua-api.factorio.com/latest/index.html is meant for control.lua scripting, not the data stage.

Got it.

The reload() method is used to reload the recipe when the prototype changes during a game (e.g. because a mod was added or a startup setting changed). What are you trying to achieve by calling it?

Probably unnecessary for a small mod like this, but for the sake of practice I wanted to make it future-proof. Say I hard-code the value to the currently documented default:

Code: Select all

local test = table.deepcopy(data.raw.recipe['vanilla-item']
if not test.energy_required then energy_required = 0.5 end

Tomorrow (I know, highly unlikely!) the devs decide to change the default value to .75 -- then I'd have to manually adapt the value. My idea was to somehow fetch the current default value automatically, but I now understand that my approach with reload() was wrong.

[eduran]

If you just want to keep the same value you don't have to define it all. That way your recipe will also use the default value and automatically update in case that ever gets changed. If you need the actual value to calculate something from it, there is no way around hard-coding it.

Code: Select all

local default_energy_required = 0.5 -- define once, easily updated if necessary

-- only use default_energy_required from now on, not the value 0.5
local recipe = table.deepcopy(data.raw.recipe['vanilla-item'])
local energy_required = recipe.energy_required or default_energy_required
-- do something with energy_required 

[eradicator]

I want to read the crafting time (energy_required) for that new item, but If I try to access data.raw.recipe['item'].energy_required from data-updates.lua, I get a nil value. However, energy_required has a default value, and that value is also used in the game. I thought I could use the value from the vanilla item, but it isn't available there either. Guess it's inherited from the prototype later on?

You are looking at the prototype, there is nothing to inherit from. The default value is used whenever a property is not defined. If you read energy_required and get nil, you know it is using the default value.

I'll know it's using the default value, but I won't know what that value will actually be (unless I look in the documentation).

Nope. Welcome to the dungeon of recipe prototypes. You know nothing yet. What you actually know is that Recipe.energy_required is not defined. But the recipe could still have Recipe.normal.energy_required and/or Recipe.expensive.energy_required. This "difficulty triplet" is applicable for almost all values a recipe can have. If you copy recipes from other mods there's always the possibility that the author changes definitions later to a different style, therefore i highly recommend writing a function around it (just a few hundred lines of highly annoying code :p.) or *gosh* hardcoding the shit....

Code: Select all

local default_energy_required = 0.5 -- define once, easily updated if necessary

Giving names to numeric values also generally makes your code much more readable. And if that is too verbose giving a formula instead of pre-calculating the result is also nice.

Code: Select all

local x = 6*60 -- six seconds
local y = 2/32 -- two pixels

[Pi-C]

Welcome to the dungeon of recipe prototypes. You know nothing yet. What you actually know is that Recipe.energy_required is not defined. But the recipe could still have Recipe.normal.energy_required and/or Recipe.expensive.energy_required. This "difficulty triplet" is applicable for almost all values a recipe can have. If you copy recipes from other mods there's always the possibility that the author changes definitions later to a different style, therefore i highly recommend writing a function around it (just a few hundred lines of highly annoying code :p.) or *gosh* hardcoding the shit....

Right, I remember reading about that somewhere …

Actually, the question is what's worse: spending some (or even a lot of) time once and never having to care about changing recipes anymore, or having to continuously look out for changes in other mods and manually change the code each time something changes? With some luck, somebody might point put relevant changes for you (e.g. bug reports), so the second case might become irrelevant. You could also assume that if there was a change and nobody has taken the time to report it, the recipe change wasn't important enough to be noticed in the first place -- and then the second case will become irrelevant.

Code: Select all

local default_energy_required = 0.5 -- define once, easily updated if necessary

Giving names to numeric values also generally makes your code much more readable.

I'm familiar with the concept of constants, and I agree: they make for much more readability!

And if that is too verbose giving a formula instead of pre-calculating the result is also nice.

Code: Select all

local x = 6*60 -- six seconds
local y = 2/32 -- two pixels

And there I was thinking "Wouldn't '60^3' be easier to understand than 216000?" when I wrote the code for on_nth_tick, fully negligent of the fact that I was using a formula.

Not to be facetious, but the other obvious solution is: pick a mod. If there are two forks of the same thing in active use, that's not really good news for anyone. One of them is likely to be more worthy of support than the other.

I've found a slightly different way: Let the user pick a mod! I warn the player (or admins) once per game session if none of the required mods is active and also if two conflicting ones are active. In case the player really wants to use both mods for whatever reason, the warning of conflicting mods can be turned off.

By the way, I've released my mod meanwhile. Thanks for the help to everybody!

[Qon]

therefore i highly recommend writing a function around it (just a few hundred lines of highly annoying code :p.) or *gosh* hardcoding the shit....

What exactly is the code supposed to do? Just reading a few values doesn't require hundreds of lines of code...

[eradicator]

I've found a slightly different way: Let the user pick a mod! I warn the player (or admins) once per game session if none of the required mods is active and also if two conflicting ones are active. In case the player really wants to use both mods for whatever reason, the warning of conflicting mods can be turned off.

Btw, trick question: If the user adds a conflicting mod to the mix (i.e. has A installed, also adds B) will that stop your mod from making prototypes? I.e....does *adding* a mod in this case lead to *removal* of prototypes? Because in that case your "Warning" would be after the migration message that tells the player all their stuff has just been deleted :D?

therefore i highly recommend writing a function around it (just a few hundred lines of highly annoying code :p.) or *gosh* hardcoding the shit....

What exactly is the code supposed to do? Just reading a few values doesn't require hundreds of lines of code...

Normalize recipes into a non-ambigious form obviously. If you think that's only a few lines of code you haven't really looked at all the stuff that recipes can contain.

[Pi-C]

I've found a slightly different way: Let the user pick a mod! I warn the player (or admins) once per game session if none of the required mods is active and also if two conflicting ones are active. In case the player really wants to use both mods for whatever reason, the warning of conflicting mods can be turned off.

Btw, trick question: If the user adds a conflicting mod to the mix (i.e. has A installed, also adds B) will that stop your mod from making prototypes? I.e....does *adding* a mod in this case lead to *removal* of prototypes?

Sincerely, I don't know -- but it also shouldn't be relevant in this case, so far. I don't make any prototypes, I only modify existing ones. So far, there are just 2 mods (aside from the one I've a hard dependency on) that are supported. They "conflict" with each other -- but the conflict doesn't break the game, only logic. Mod A replaces the vanilla lamp with an inlaid lamp, mod B enables another inlaid lamp (same recipe, same graphics, same behavior, but different entity name) after research that coexists with the vanilla lamp. But as the vanilla lamp has already been modified by A, this new item is redundant.

Nevertheless, it could be reasonable to have both mods active at the same time: A makes sure that all lamps will be inlaid lamps, so you won't have to worry about unmodified vanilla lamps hidden somewhere on the map. B also provides a bigger version of the inlaid lamp, which might be considered useful by some players.

Because in that case your "Warning" would be after the migration message that tells the player all their stuff has just been deleted ?

I've thought about adding a new entity (a bigger inlaid UV-lamp) and removing the recipe from B if A was present. What you wrote is exactly the reason I didn't do it in the end.

[Adamo]

I do this often in my mods. You have to write code to handle it. Make any mods that may or may not appear optional using ?, then you'll need to write some if statements in your data.lua code to determine who is serving what, and decide where things should go. You can do anything you want at this stage, including completely overwriting existing data from those mods, if you want. So the answer is yes you can absolutely do what you want to do, but it needs to be handled through problem solving statements in your data.lua (and data-updated and data-final-fixes) code.

I'll give you an example. Here is a piece of code I have for a chemistry mod. It optionally requires a number of mods that I know add chemistry recipes to the game. It doesn't even ask about particular mods, although you could do that with the api, ask for them by name, instead, this code just trolls the recipes, whatever is available from any mod, and copies the recipes into new recipes that work with my chemistry buildings to make the same things.

Code: Select all

require("util")

function add_recipe_to_tech(tech_name,recipe_name)
	if data.raw["technology"][tech_name] then
		table.insert(data.raw["technology"][tech_name].effects, {
			type="unlock-recipe",
			recipe=recipe_name
		})
	end
end

for _,recipe in pairs(data.raw.recipe) do
	local skip_recipe = true
	if recipe.category == "chemistry" then
		skip_recipe = false
    elseif recipe.category == "drugs" then
    	skip_recipe = false
    elseif recipe.category == "drugs-et-et" then
    	skip_recipe = false
    elseif recipe.category == "drugs-to-et" then
    	skip_recipe = false
    elseif recipe.category == "drugs-et-to" then
    	skip_recipe = false
    elseif recipe.category == "drugs-to-to" then
    	skip_recipe = false
    elseif recipe.category == "drugs-tre-tre" then
    	skip_recipe = false
    elseif recipe.category == "agitation" then
    	skip_recipe = false
    elseif recipe.category == "advanced-agitation" then
    	skip_recipe = false
    elseif recipe.category == "drugged-science" then
    	skip_recipe = false
	end
	local solid_input_count = 0
	if not skip_recipe and recipe.ingredients then
		for k,ingredient in pairs(recipe.ingredients) do
			if ingredient.type ~= "fluid" then
				solid_input_count = solid_input_count + 1
			end
		end
	end
	local result
	local new_category
	if not skip_recipe
	and type(recipe.results) == "table" then
		for k,result in pairs(recipe.results) do
			if result.type ~= "fluid" then
				skip_recipe = true
				break
			end
		end
		if not skip_recipe then
			result = recipe.results[1]
			if recipe.results[3] then
				new_category = "adamo-chemical-pressurized-reaction"
			else
				if solid_input_count == 0 then
					new_category = "adamo-chemical-distillation"
				else
					new_category = "adamo-chemical-reaction"
				end
			end
		end
	else
		skip_recipe = true
	end
	if not skip_recipe then
		local new_recipe = util.table.deepcopy(recipe)
		new_recipe.name = "adamo-chemical-copy-"..recipe.name
		if result then
			new_recipe.main_product = result.name
		end
		new_recipe.category = new_category
		new_recipe.subgroup = new_category
		data:extend({new_recipe})
		for k,tech in pairs(data.raw.technology) do
			local found_tech = false
			if tech.effects then 
				for l,effect in pairs(tech.effects) do
					if effect.type == "unlock-recipe"
					and effect.recipe == recipe.name then
						add_recipe_to_tech(tech.name,new_recipe.name)
					end
				end
			end
		end
	end
end