Comments (13)
For GUI refactoring this check
if gui_element and gui_element.valid then
to be inside the handler calling loop would fix that
from factorio-stdlib.
Is this actually a valid issue? The stdlib is mostly a transparent pass-through of events so how would it ever receive an event for an entity or element that was not valid?
from factorio-stdlib.
It doesn't receive it directly but if there are multiple handlers registered for an event, handler 1 could invalidate the entity and now handler 2 would get passed the invalid entity.
Since Factorio does not raise the event for invalid entities checking for .valid on entities passed in the event is not required.
from factorio-stdlib.
Here is something I cooked up but I am not sure how much I like it or the impact it would have on performance.
function Event.dispatch(event)
Core.fail_if_missing(event, "missing event argument")
if Event._registry[event.name] then
for _, handler in pairs(Event._registry[event.name]) do
if not table.any(event, function(v) return type(v) == "table" and type(v.__self) == "userdata" and not v.valid end) then
local metatbl = { __index = function(tbl, key) if key == '_handler' then return handler else return rawget(tbl, key) end end }
setmetatable(event, metatbl)
local success, err = pcall(handler, event)
from factorio-stdlib.
It doesn't receive it directly but if there are multiple handlers registered for an event, handler 1 could invalidate the entity and now handler 2 would get passed the invalid entity.
I agree that is an edge-case that occurs in stdlib events and might not occur in the regular event handlers. I am concerned that automatically "filtering" these events also introduces a hidden factor that could be confusing to developers. Filtering the invalid events with invalid entities could lead to events seeming to "disappear" after appearing for only one event handler. I fully expect to fail to read the documentation encounter this at some point. And I'll probably spend hours trying to debug it.
The opposite edge-case, letting events fire with invalid entities is also bad - but perhaps less confusingly so to developers. Likely this extra event will lead to an error when the developer did not check .valid
, but this error is very visible in contrast to the hidden event edge-case. So barring a better third-solution here, I'm inclined to not filter invalid events out.
from factorio-stdlib.
Giving the complexity of both Factorio and stdlib events, reading the manual is pretty much required. I understand where you are coming from but following the same practices as Factorio itself by not passing along events for invalid userdata seems the best approach. A good compromise between both could be adding a log() event when stdlib Event tries to pass invalid event data.
from factorio-stdlib.
I understand where you are coming from but following the same practices as Factorio itself by not passing along events for invalid userdata seems the best approach.
Actually, how does Factorio handle this sort of behavior? I haven't checked, but something analogous to this should be possible to set up, at least between multiple mods. If two mods are registered to an event, and the first mod destroys the entity related to the event, does the second mod receive the event? Do you know what behavior happens here?
from factorio-stdlib.
The second mod will not receive the event. All Factorio raised events are guaranteed to be valid.
Mod raised events can still script.raise_event() invalid stuff
I can't find the exact post but this is slightly related https://forums.factorio.com/viewtopic.php?f=7&t=25473
from factorio-stdlib.
Well if that is how factorio is handling it, then stdlib will have to follow suit. I am vaguely concerned about the example condition you suggested though:
table.any(event, function(v) return type(v) == "table" and type(v.__self) == "userdata" and not v.valid end)
this concerns me because it broadly rejects any event that contains an invalid userdata table. The assumption here is that no game events pass in invalid userdata objects intentionally... and I'm not sure that's actually true.
from factorio-stdlib.
https://forums.factorio.com/viewtopic.php?t=32039#p202158
from factorio-stdlib.
Good to know... For future reference, can you link to a that forum post in a comment in the code as well?
from factorio-stdlib.
Still not sure if I like table.any for this maybe limiting it to entities would be the better choice. It won't catch them all but should catch the majority of events.
local entity = event.created_entity or event.entity or event.source or event.destination or event.last_entity
if entity.valid then
from factorio-stdlib.
I don't think the giant or clause is a great idea either (what if there are two entities and the first is valid and the second is not?).
If you want to make a table of fields, you could iterate that instead:
local fields = { "created_entity", "entity", "source", "destination", "last_entity" }
if table.any(fields, function(field) return event[field] and not event[field].valid end) then
from factorio-stdlib.
Related Issues (20)
- Upgrade Readme and Wiki HOT 2
- To Do List
- Data.Util.Duplicate on normal/expensive recipes HOT 6
- Wiki with list of functions and their functionalities HOT 2
- Recipe:clear_ingredients error HOT 5
- Small errors found with trains/events HOT 6
- add_ingredient issue HOT 8
- 0.18 support? HOT 1
- remove_prereq nullifies prerequisites value of technology table.
- errors found with trains/events
- add_ingredient cant find items that are modules
- Event handler filter is ignored if more than one event ID is specified in `register`
- spidertron-remotes aren't checked as "items" so a recipe result (and more) may fail to find one HOT 1
- documentation broken HOT 2
- Event.register not passing pattern to function HOT 1
- Inconsistency of technology pack methods
- Gui events not working HOT 1
- String.contains doesn't work with strings that contain a hyphen.
- Question: Which tool can be used to interpret the code documentation in VSCode
- This does not look right: return (Area.size(Area)
Recommend Projects
-
React
A declarative, efficient, and flexible JavaScript library for building user interfaces.
-
Vue.js
🖖 Vue.js is a progressive, incrementally-adoptable JavaScript framework for building UI on the web.
-
Typescript
TypeScript is a superset of JavaScript that compiles to clean JavaScript output.
-
TensorFlow
An Open Source Machine Learning Framework for Everyone
-
Django
The Web framework for perfectionists with deadlines.
-
Laravel
A PHP framework for web artisans
-
D3
Bring data to life with SVG, Canvas and HTML. 📊📈🎉
-
Recommend Topics
-
javascript
JavaScript (JS) is a lightweight interpreted programming language with first-class functions.
-
web
Some thing interesting about web. New door for the world.
-
server
A server is a program made to process requests and deliver data to clients.
-
Machine learning
Machine learning is a way of modeling and interpreting data that allows a piece of software to respond intelligently.
-
Visualization
Some thing interesting about visualization, use data art
-
Game
Some thing interesting about game, make everyone happy.
Recommend Org
-
Facebook
We are working to build community through open source technology. NB: members must have two-factor auth.
-
Microsoft
Open source projects and samples from Microsoft.
-
Google
Google ❤️ Open Source for everyone.
-
Alibaba
Alibaba Open Source for everyone
-
D3
Data-Driven Documents codes.
-
Tencent
China tencent open source team.
from factorio-stdlib.