Mod Thoughts and Feedback

[simplex]

@PeterA or @MarkL

What are the bit sizes of net_smallbyte and net_tinybyte?

I initially assumed net_smallbyte was a nibble, hence 4 bits long. But then logic dictates net_tinybyte would be strictly shorter than 4 bits long. However, a net_tinybyte is used to store the moon phase in components/clock.lua, and since there are 5 moon phases (and 2^2 = 4 < 5), a net_tinybyte must necessarily be at least 3 bits long.

I'm stumped because since this fractional byte data must be packed into whole bytes before transmitting through the network, their bit sizes should ideally be divisors of 8, but by the above this could only happen if either:

• net_smallbyte is 8 bits long and net_tinybyte is 4 bits long; this is silly because then a net_smallbyte would be the same as a net_byte.
• both net_smallbyte and net_tinybyte are 4 bits long; this is also silly.

Please enlighten me.

Also, can the integer types other than net_uint and net_ushortint hold signed values? If so, is the sign bit reserved (so that, for instance, the highest positive value net_byte can hold is 2^7 - 1 = 127)?

Edited December 16, 2014 by simplex

[StalkinU]

In addition, there are certain things in the game that are rare or restricted, and we want to maintain that. For example, owners of Reign of Giants can play as Wigfried and Webber, and we’d like to leave it exclusive to the owners of the DLC to be able to do that.

 

---messed up the quote XD---

 

Just checked, I can't play as webber or wigfried.

Is there a way for us modders to make the mod check for the DLC before enabling both of them?

Edited December 16, 2014 by StalkinU

[simplex]

@PeterA

Could you expose a way for mods to add custom RPC requests/handlers? Preferably nesting the new RPC codes under the mod name to prevent clashes, much like what is done with mod actions?

[Rincevvind]

i didn't own DST, but want to ask - is it possible to use mods without requirement to have it for other players? i mean there some modes which don't affect game, like interface mods. Changing form/position of status bars isn't gamebreaking, so why all must have same UI?

[rezecib]

@Rincevvind, Yes, there are a few new flags in the modinfo:

dst_compatible = true--This lets clients know if they need to get the mod from the Steam Workshop to join the gameall_clients_require_mod = false--Basically the opposite of the aboveclients_only_mod = false

[simplex]

@PeterA

Could you allow for mods specifying more than one compatible api_version? This has been requested time and time again over the lifespan of base DS and RoG, but now it is more relevant than ever, since singleplayer DS had its api_version frozen at 6, whilst DST starts at 10. Since mods "from the future" (i.e., with a api_version version higher than the game specifies) are not allowed to load, in order to get a mod cross-compatible with DS and DST the only choice is to set api_version to 6; but then, DST will claim it is outdated, which has always been a PR nightmare.

The simpler approach would be to add an optional dst_api_version field to modinfo, which is used instead of api_version under DST if present. The more versatile approach would be to allow api_version to be a table, using as the "effective" api_version its highest entry which does not exceed the game's MOD_API_VERSION; but, of course, the latter approach would require patching singleplayer DS as well.

[rezecib]

@simplex, Is it not possible to run the IsDST check you wrote like this?

api_version = IsDST() and 10 or 6

Although I agree with you that a dst_api_version to allow specification of both more easily would be better.

[Kzisor]

@simplex, Is it not possible to run the IsDST check you wrote like this?

api_version = IsDST() and 10 or 6

Although I agree with you that a dst_api_version to allow specification of both more easily would be better.

 

Functions cannot be ran from within modinfo.lua I've already tried that approach.

[simplex]

@simplex, Is it not possible to run the IsDST check you wrote like this?

api_version = IsDST() and 10 or 6

Although I agree with you that a dst_api_version to allow specification of both more easily would be better.

As Kzisor said, modinfo.lua runs in an empty environment, so we can't perform any checks of that sort. For that we have a hard dependence on what Klei provides us in the backend.

EDIT: I had misspelled Kzisor's name, so the mention was blank.

Edited December 20, 2014 by simplex

[simplex]

@PeterA

Could you extend the modinfo.lua spec by requiring an entry to serve as a universally unique identifier for networking purposes (such as the 'mod_name' entry in actions)? Currently the 'modname' variable in the mod's environment (which holds the mod's directory name) is used, but this presents an obvious issue in that if not all people on the server have the same mod directory name (such as not all of them having downloaded it from the workshop) then networking will simply break. My suggestion is the inclusion of a required "id" field in modinfo.lua, which should be a unique string identifying the mod (unlike the 'name' field, which is a matter of user presentation, and can certainly change, as well as clash with other mods). When loading mods, the 'id' fields should be checked for uniqueness, raising an error if two enabled mods have the same id.

For more on this topic, please check this discussion, starting from the linked post.

[Rincevvind]

and about id, there can be common rule for name like author_modname, so similar names never clash

id=simplex_showmapid=xxxNOOBKILLAHxxx_showmap

[simplex]

and about id, there can be common rule for name like author_modname, so similar names never clash

 

id=simplex_showmapid=xxxNOOBKILLAHxxx_showmap

There's no need to go all Java about namespaces. C++ library namespaces don't prepend author/domain to their id and they work just fine. A mod author can check the ids of similarly named mods, if any, and if not some user should certainly submit a crash report with the non-unique id error.

The reason I'd like to avoid overextending the id is for network performance reasons. Whenever custom mod data is transmitted through the network (such as a custom action to be performed), the mod's unique id (currently the directory name) is transmitted with it. So, if a mod makes heavy use of such networking, a longer id is bound to incur a significant bandwidth overhead.

Edited December 20, 2014 by simplex

[simplex]

@PeterA

Could you make the RPC calls sending recipe information identify the sent recipe by its name, as a string, instead of its numerical sortkey? The RPC handlers using recipe identification are just MakeRecipeFromMenu, MakeRecipeAtPoint and BufferBuild, which are very seldom sent network events, so the network performance impact should be negligible.

I ask because, currently, even ignoring the obvious aesthetical implication that mods can no longer customize their recipe orders in the crafting menu, proper network communication of recipes relies on their addition order, which means client/server communication may break if a client has mods with a different loading order than the host (this is a particularly severe concern form mods which don't specify a loading priority, i.e. the majority of them).

[Some1]

@PeterA
 
Сould you also extend the modinfo environment with the language variable, taken from the LanguageTranslator.defaultlang?
 
It would be very handy to have that native support to make different language descriptions (and configuration_options) depending on specific game language.

This can be used as follows:

 

http://pastebin.com/Q1QfDapX

(The built-in syntax highlither messed all russian symbols up).

Edited December 22, 2014 by Some1

[PeterA]

which means client/server communication may break if a client has mods with a different loading order than the host (this is a particularly severe concern form mods which don't specify a loading priority, i.e. the majority of them).

Yeah, as we improve systems to be mod supported, one of our requirements is to ensure that they will be functional with varying mod configurations(and load order) so that inter-mod compatibility is maintained.

[rezecib]

The solution I've been using so far for recipes has been to set the sortkey to the negative of the sortkey it was assigned. Doesn't allow intermixing the new recipes with the default ones, but it does let you set an ordering above and below (if you leave it with the assigned sortkey) the default recipes.

[simplex]

The solution I've been using so far for recipes has been to set the sortkey to the negative of the sortkey it was assigned. Doesn't allow intermixing the new recipes with the default ones, but it does let you set an ordering above and below (if you leave it with the assigned sortkey) the default recipes.

I'm more concerned with the world not exploding due to mismatched ids between client and server due to differences in mod loading order than with customising the sorting position in the crafting tab (though that'd certainly be a bonus).

[Some1]

Why not sort all mods by names, and load them accordingly?

[rezecib]

@simplex, Well, the negative sortkey method (+all_clients_require_mod, which you definitely need for any mod that involves recipes) seems to work for that. 

 

Nevermind, I see the problem now. Mods could load in a different order on the client than on the server, resulting in a different (negative or positive) sortkey. But as for exploding, I don't think that happens, but instead people just can't make certain recipes, or trying to make certain ones ends up making different ones.

 

Some1's method could work too, though. That is, if it sorts by name within sets of mods with the same priority.

Edited December 22, 2014 by rezecib

[simplex]

Nevermind, I see the problem now. Mods could load in a different order on the client than on the server, resulting in a different (negative or positive) sortkey. But as for exploding, I don't think that happens, but instead people just can't make certain recipes, or trying to make certain ones ends up making different ones.

Yes, the world wouldn't literally explode, I was just using colourful language. But clicking on one recipe and making another should be enough to be called game breaking.

 

Some1's method could work too, though. That is, if it sorts by name within sets of mods with the same priority.

It'd reduce the likelihood of issues, yes, but there are still some corner cases to consider (and probably some more unexpected ones). For example, suppose a server has one or more mods providing in total more than one characters with custom recipes added in their constructors. Suppose also at least two of such characters are being played simultaneously. Can the order in which these additional recipes are added on the host be the same as the order in which they are added in the clients playing the characters (i.e., are the character entities ensured to spawn in the same order everywhere)? If not, we'll get back to the same issue regardless of mod loading order.

I see no reason why sortkeys should be used as recipe network identifiers instead of recipe names. Recipe names are unique, and had served so far (in singleplayer) as the id for recipes. Recipes are not made so often for the bandwidth save in transmitting a number instead of a string to be significant.

If we assume numerical RPC args are serialised as single precision IEEE floats, they take 4 bytes. The average size of a recipe name is 10.04, so that'd be 6.04 extra bytes* per recipe related RPC call. For that to account for even a 1KB/s bandwidth usage increase, a client would need to make ~170 recipes per second**. There's no reason to not adopt the greater robustness in using recipe names, and making the switch would take minimal adjustments to the game code (namely, it'd take passing "recipe.name" where "recipe.sortkey" is passed to RPCs, and on receiving the RPCs the loop searching for the target recipe in AllRecipes would be replaced by just indexing AllRecipes with the received name).

* In practice, it'd actually be either 7.04 or 10.04, depending on which string serialisation method is used (sentinel character vs prefixed length), but let's not get too nitpicky.

** This is based on the simplified mathematical assumption that all recipes are crafted with the same probability, but regardless of that the principle still stands. The largest recipe name is 21 bytes long, so we get a lower bound of ~60.2 recipes per second, or ~48.8 recipes per second if we include the additional 4 bytes from the most prodigal serialisation method (prefixed length).

Edited December 22, 2014 by simplex

[rezecib]

It'd reduce the likelihood of issues, yes, but there are still some corner cases to consider (and probably some more unexpected ones). For example, suppose a server has one or more mods providing in total more than one characters with custom recipes added in their constructors. Suppose also at least two of such characters are being played simultaneously. Can the order in which these additional recipes are added on the host be the same as the order in which they are added in the clients playing the characters (i.e., are the character entities ensured to spawn in the same order everywhere)? If not, we'll get back to the same issue regardless of mod loading order.

 The game's code no longer specifies any character-specific recipes/recipetabs in the character constructors. So I'd assume that we should rewrite that part to be handled a different way. But if your method would also remove the need for that, it would help a ton with porting DS characters.

 

But yeah, labeling it by mod name definitely seems like the more robust method, and as you point out, recipe uses are hardly relevant in terms of the scale of network usage.

[simplex]

The game's code no longer specifies any character-specific recipes/recipetabs in the character constructors. So I'd assume that we should rewrite that part to be handled a different way. But if your method would also remove the need for that, it would help a ton with porting DS characters.

Well, currently the game itself would only have use for character specific recipes for wickerbottom, and it handles that instead by giving it the "bookbuilder" tag. Then widgets/crafttabs.lua checks for that tag and only shows the book recipes if it's present. So all characters have wickerbottom recipes, they're just not visible to most. The game can get away with that because widgets/crafttabs.lua was customized specifically for it; it's a hardcoded check, not really applicable to mod characters unless you'd like to pretty much overwrite the CraftTabs constructor... And it only works cleanly because the recipes are in a dedicated tab, to account for cases like Wigfrid it'd get messy pretty quick. For modded characters, the cleanest method still is, and probably will still be, adding recipes in the prefab constructor.

But yeah, labeling it by mod name definitely seems like the more robust method, and as you point out, recipe uses are hardly relevant in terms of the scale of network usage.

It's not really about labelling by mod name, it's about removing the need to label by mods altogether. Recipe names are necessarily unique (otherwise they'd overwrite another recipe when added to the AllRecipes table). Keep using sortkeys and also including the mod's name, similarly to how mod actions are handled, would be an alternate solution, but it'd not only be (a bit) more complex to implement but also less extensible, not giving us back full customisability over sortkeys, for instance. It'd also still give rise to the issue I presented before with mod character recipes if multiple characters are present in the same mod. Using recipe names as network identifiers would be the simplest, more flexible and less bug prone approach to mod friendliness.

[rezecib]

@simplex, This is the system I used:

local OldIsRecipeValid = GLOBAL.IsRecipeValidlocal function IsRecipeValid(recipe)	return OldIsRecipeValid(recipe) and		((GLOBAL.ThePlayer and GLOBAL.ThePlayer:HasTag(recipe.name.."_builder")) or not recipe.tagneeded)endGLOBAL.IsRecipeValid = IsRecipeValid

Then, in the character prefabs I add the tag "spidereggsack_builder", etc, and declared the recipes in the modmain. This avoids rewriting crafttabs, but still does the hiding/showing of recipes per character.

[simplex]

@PeterA

Could you add a DoAction RPC handler that receives a remote action to be performed by the player? It could work exactly like DoWidgetButtonAction (minus the container component check), by replacing the final

BufferedAction(player, target, action):Do()

with

player.components.locomotor:PushAction(BufferedAction(player, target, action))

though being able to specify the BufferedAction's invobject and pos constructor parameters would be a welcome bonus.

[simplex]

@PeterA

Could you add a DoAction RPC handler that receives a remote action to be performed by the player? It could work exactly like DoWidgetButtonAction (minus the container component check), by replacing the final

BufferedAction(player, target, action):Do()

with

player.components.locomotor:PushAction(BufferedAction(player, target, action))

though being able to specify the BufferedAction's invobject and pos constructor parameters would be a welcome bonus.

And could you also add some netvar broadcasting to clients over whether a player's buffered action failed or completed successfully (by "failed", I mean the "actionfailed" event was pushed, and by "completed successfully" I mean EntityScript:PerformBufferdAction() returned true; the latter case used to push the event "actionsuccess" in DS, but no more)? And could you also store in a netvar whether the player is busy (i.e., its stategraph has either the tag "busy" or "working"). I know this is a bit of a tall request, but my goal is to port Action Queue as a clientside only mod, since it's essentially a matter of interface; currently I'll have to include some supporting serverside logic.

EDIT: This was supposed to be an edit of my previous post, not a reply to it, but what's done is done.

Edited December 26, 2014 by simplex