on_player_opened/on_player_closed

Supercheese
Filter Inserter
Filter Inserter
Posts: 841
Joined: Mon Sep 14, 2015 7:40 am
Contact:

Re: on_player_opened/on_player_closed

Post by Supercheese »

Mooncat wrote:I don't like this. Adding an additional item and requiring an additional user action for just a simple thing is quite bad. I would prefer on_tick rather than the wrench.
I agree, it's far from optimal.
mknejp
Fast Inserter
Fast Inserter
Posts: 154
Joined: Wed Apr 27, 2016 8:29 pm
Contact:

Re: on_player_opened/on_player_closed

Post by mknejp »

Rseding91 wrote: A simple example:

1. The player has a blueprint open to edit that's contained in a chest.
2. A biter destroys the chest which goes through and clears every item in the chest 1 by 1
3. In reaction to the blueprint being destroyed the GUI is closed
4. In reaction to being closed the event is fired
5. The mod does some stuff and one of the things it does is delete the chest that's being killed
6. Returning back to the biter destroying the chest: it's now in-process of destroying items but they're all already destroyed and it's in an invalid state and crashes

You take that same scenario and apply it pretty much everywhere except the player pressing "E" to close the GUI and there's just no simple answer to making this work yet.
That must be the most convoluted and unstructured game update loop I have ever seen. Doesn't the game update its state in phases? Something like: record all state changes, process state changes, apply changes, fire internal events, apply changes from internal events, fire delayed mod events, apply changes from mod events. It's no wonder interleaving data and logic processing as shown above over-complicates the code with sanity checks everywhere. That kind of complexity is prone to cause problems/bugs in all kind of places.
Rseding91
Factorio Staff
Factorio Staff
Posts: 14343
Joined: Wed Jun 11, 2014 5:23 am
Contact:

Re: on_player_opened/on_player_closed

Post by Rseding91 »

mknejp wrote:That must be the most convoluted and unstructured game update loop I have ever seen. Doesn't the game update its state in phases? Something like: record all state changes, process state changes, apply changes, fire internal events, apply changes from internal events, fire delayed mod events, apply changes from mod events. It's no wonder interleaving data and logic processing as shown above over-complicates the code with sanity checks everywhere. That kind of complexity is prone to cause problems/bugs in all kind of places.
No, that would add a large amount of overhead slowing the game down unnecessarily. It also wouldn't work because you can't have an "entity died" event after the entity is long gone - as one simple example.

There aren't issues with our internal code because we simply don't write bad logic that mods are free to do all day long.
If you want to get ahold of me I'm almost always on Discord.
User avatar
binbinhfr
Smart Inserter
Smart Inserter
Posts: 1524
Joined: Sat Feb 27, 2016 7:37 pm
Contact:

Re: on_player_opened/on_player_closed

Post by binbinhfr »

Rseding91 wrote:There aren't issues with our internal code because we simply don't write bad logic that mods are free to do all day long.
That's what I was suspected : the factorio team does not like modders :-D
It's good for their game because players love customisation, but it complicates their work a lot.
And, of course, I can understand that.

Anyway, if I try a mod, and it does not work because crash or error-bugs, I simply uninstall it (or give a chance to the modder by reporting the bug if I think the mod deserves it). So mods are free to do bad logic, but at the end these ones won't be used... So are you really receiving mods problems report all day long ???
My mods on the Factorio Mod Portal :geek:
mknejp
Fast Inserter
Fast Inserter
Posts: 154
Joined: Wed Apr 27, 2016 8:29 pm
Contact:

Re: on_player_opened/on_player_closed

Post by mknejp »

Rseding91 wrote:No, that would add a large amount of overhead slowing the game down unnecessarily.
That is very questionable. The whole point of delaying the events is to improve performance by grouping data that needs the same logic. Processing identical operations and data in batches is the very reason big games today can exploit the hardware the way they do and run at interactive frame rates.
Rseding91 wrote:It also wouldn't work because you can't have an "entity died" event after the entity is long gone - as one simple example.
But of course you can. Delaying events makes this work. When processing the data the game keeps track of all the entities that have died this frame and marks them as dead, and when done processing it goes through the event listeners for the respective events. Only after that are entities actually removed from the system. This delaying of critical, non-parallel events is common practice to avoid cache stalling in the middle of a tight loop. And every component update should be considered part of a tight loop otherwise something's wrong. At least in a data oriented architecture that aims at maximum exploitation of what the hardware has to offer. But in such an architecture one has to turn their mind upside down and understand that it's not about the objects but the data transformations that have to happen. And a game, like all software in existence, is just a means of transforming data from one form to another. These designs are intended for maximum throughput. That's also why I would never intertwine game update code with external mod code. Allowing that in the first place is the reason for all those safety checks. By delaying the mod code to the end of the frame where most of the work is done and internal consistency is restored a mod can do whatever they want without risk of breaking invariants in the game's data update cycle.
Rseding91 wrote:There aren't issues with our internal code because we simply don't write bad logic that mods are free to do all day long.
Is "we simply don't make mistakes" really an honest solution? Does that imply it cannot be improved? Events firing left and right throughout the whole entity update cycle instead of at exactly defined moments is a recipe for bugs and headaches as you described. In such an environment it is very easy to accidentally introduce bugs simply because some event listener has accidental dependency on certain entity updates/events happening in a certain order. Those bugs are the hardest to find and test for. In data oriented architectures such problems simply don't exist because every phase of the game update is a well defined functional step with inputs and outputs that cannot suddenly change performance characteristics or have unintended side effects. They are among the easiest to write, test and reason about.

Btw it usually becomes "bad logic" only after it was found to have a bug :)
Rseding91
Factorio Staff
Factorio Staff
Posts: 14343
Joined: Wed Jun 11, 2014 5:23 am
Contact:

Re: on_player_opened/on_player_closed

Post by Rseding91 »

That's a lot of words but... have you actually made something that runs at the scale Factorio does with the performance Factorio does with as little bugs as Factorio has?
If you want to get ahold of me I'm almost always on Discord.
mknejp
Fast Inserter
Fast Inserter
Posts: 154
Joined: Wed Apr 27, 2016 8:29 pm
Contact:

Re: on_player_opened/on_player_closed

Post by mknejp »

Rseding91 wrote:That's a lot of words but... have you actually made something that runs at the scale Factorio does with the performance Factorio does with as little bugs as Factorio has?
I don't know what the "scale" argument is meant to be. If it's just the number of entities then no, but it's also a pretty much irrelevant metric. What matters is how much work has to be done every frame and how efficient it is in terms of using the available resources. In that regard you can have lots of entities that each do little or fewer entities that have highly complex behavior. If a game peaks at 100% CPU utilization on all cores and has fully saturated memory bandwidth while being efficient (i.e. doing the least amount of work necessary to do the job) then there is nothing that can improve it besides better hardware.

What I do know however is that any game (or any other simulation or performance critical software) developed these days that has more than basic complexity cannot utilize its full potential with an OOP architecture. That's just a reality of how hardware works.

And, we only get to see the bugs that make it into the public releases. There's no way of knowing how much time is spent fixing bugs that we never get to see :)

Don't get me wrong. I like the game otherwise I wouldn't be here and I am an optimization person so I have these things in my head all the time in and out of work and I like talking about it (probably more vocally than I should - people hate that about me). I know that may come across the wrong way. It just saddens me to see software not being as hardware efficient as it could be. People these days tend to more and more think that throwing hardware at a problem fixes everything even though ironically that is becoming less and less true. That's why it's perplexing to me when algorithms and data structures that were invented to make games go faster on current hardware architectures are rejected with "that'll make it worse". It's really confusing to me and I'd like to understand the reasoning behind it. I'd love to see the code to understand why it's apparently so different and whether it that can be improved. Looking inside the box so to say.
User avatar
Mooncat
Smart Inserter
Smart Inserter
Posts: 1196
Joined: Wed May 18, 2016 4:55 pm
Contact:

Re: on_player_opened/on_player_closed

Post by Mooncat »

I feel some tension raised... :?

Let's have a little break first... ;)

Now at least we have a reason why these events have not been implemented: the game is so optimised to a point where calling these events without crashing the game if they are misused is difficult.

To be fair, Factorio does have very good performance. I wondered what dark magic the developers have used when I first saw many almost-compressed transport belts running simultaneously without lagging. Yes, the internal code may be unclean and not modder-friendly either, but maybe it is the reason why Factorio is so complete even though it is still in early access.

What mknejp has said is also true: there is always room for improvement, as it is also suggested in FFF-148. But let's not push the developers too hard. Even if they want to improve the engine, and clearly they do, it will take time. And also don't forget that modding support should be just a side mission. Their main objective should be completing the game itself, making it more fun to play so that many famous mods will become redundant. :lol: (Terraria is a good example on this topic I guess. Its modding support was implemented very lately but still it is a famous game.)


OK, now, turns back to the original problem... while hoping that the developers will find a way to implement the events safely, how about the suggestion from Nexela? It will only involve on_player_opened and so it should be easier to implement, or not? :mrgreen:
Nexela
Smart Inserter
Smart Inserter
Posts: 1828
Joined: Wed May 25, 2016 11:09 am
Contact:

Re: on_player_opened/on_player_closed

Post by Nexela »

I also have another wild idea

Still requires the on_player_opened

when creating a gui we add a destroyer to it. Anytime that entities gui is closed the gui is destroyed :)

game.player.gui.top.add{type="label", name="greeting", caption="Hi", destroyer="entity-name"}


event though I would still like to attach to factorio opened gui's
User avatar
aubergine18
Smart Inserter
Smart Inserter
Posts: 1264
Joined: Fri Jul 22, 2016 8:51 pm
Contact:

Re: on_player_opened/on_player_closed

Post by aubergine18 »

If an entity dies (for whatever reason) could the .valid property on it's default GUI dialog be set to false immediately? That way those of us using the tick event to update GUI can check the root element of the dialog first to see if it's still valid (if not, we bail out).
Better forum search for modders: Enclose your search term in quotes, eg. "font_color" or "custom-input" - it prevents the forum search from splitting on hypens and underscores, resulting in much more accurate results.
User avatar
Mooncat
Smart Inserter
Smart Inserter
Posts: 1196
Joined: Wed May 18, 2016 4:55 pm
Contact:

Re: on_player_opened/on_player_closed

Post by Mooncat »

I have just read the newest FFF (151)
"Gui reskin and possible improvements"

Filled with hopes and imaginations. :P
Best of luck to the devs!
User avatar
aubergine18
Smart Inserter
Smart Inserter
Posts: 1264
Joined: Fri Jul 22, 2016 8:51 pm
Contact:

Re: on_player_opened/on_player_closed

Post by aubergine18 »

Just ran in to this issue again in another mod I'm working on. So much pain. In event handlers, modders can check entity.valid before doing anything with the entity - would that put devs minds at ease when considering adding the events? An example could be provided in the api docs so modders have no excuse to forget the .valid check.

In related news: Still cringing that I can't create custom UI for my custom entities, or modify/replace UI for vanilla entities.

If extra encouragement is required to implement these events, please observe the workarounds we're currently forced to do in an on_tick handler: https://github.com/aubergine10/CircuitA ... events.lua
Better forum search for modders: Enclose your search term in quotes, eg. "font_color" or "custom-input" - it prevents the forum search from splitting on hypens and underscores, resulting in much more accurate results.
Boodals
Fast Inserter
Fast Inserter
Posts: 129
Joined: Sun Feb 11, 2018 7:10 pm
Contact:

Re: on_player_opened/on_player_closed

Post by Boodals »

This was implemented a long time ago.
Post Reply

Return to “Implemented mod requests”