KeinNiemand / Factorio-GhostOnWater

Factorio mod to make it possible to paste blueprints on water in one go
https://mods.factorio.com/mod/GhostOnWater
MIT License
1 stars 8 forks source link

Slight incompatibility with Space Explorations Constrcution Pylons #21

Closed ChaosSaber closed 1 year ago

ChaosSaber commented 1 year ago

Space Exploration has an item called Constrcution Pylon and Radar Constrcution Pylon. These are basically a roboprt and a power pole at once. If you blueprint a single one, then the blueprint actually shows, that it requires two of them. Without #20 only one of them has a dummy. With that PR both have a dummy.
Issue happens in both versions. Everything works fine when placed on land. But when placed on water, it will break, because robots will place at least one of the ghost. If that happens before the landfill is placed, then it will stay forever a dummy item and the other ghost will also stay there forever.
Probably happens with every multi-entity (or however the technical term is for these).

KeinNiemand commented 1 year ago

Multi entity from other mods work because in other mods only one of the entitles is actually blueprinted has a placable_by item (or in LTN case it uses dummy items for the station signal in out combinator), didn't look into the code of any of these mods but the mod probably spawns in the secondary entices ones the main one has been placed or something like that. Yes for some reason here the secondary entity which probably has no collision (or at least no collision with water) has a placable_by value which means it can be build by bots directly, but it presumably still builds the other entity automatically once one of them has been build something that doesn't work on water. I don't know why Space Exploration has both entitles placeable, maybe so things like connected fires including stuff like the settings to read bot network content get preserved in blueprints. Assuming I'm right about why this dosn't work the only solution I can think of is to change the placable_by item of the entity without collision to some kind of dummy item (simmilar to how LTN station input/output combinator), that would mean that BPs no longer show you requiring two of them (not sure if that happens but it would also prevent two bots from trying to place the pylon on land). And that only works if the placable_by of the second entity really only is there so it can be in BPs for preserving settings. And while I could make that change, to me it seems like that the proper place to fix this would be in space exploration itself at least from a purley technical point of view. It's not really SE job to be compatible so I'll look into this and see if I can fix it. Anyway this is just my theory, I havn't done any extensive testing/debugging or looked into SEs code so I can't tell for sure.

ChaosSaber commented 1 year ago

I searched in the SE discord and it seems to be intended. This is the answer:

Construction pylons are a roboport-electric pole composite, while radar construction pylons are roboport-radar-electric pole composites. It is possible to make some components non-blueprintable. That's why cargo rocket silos only show up in blueprints as a single entity, even though, there's a lot of component entities. In the case of construction pylons, the graphics are attached to the roboport entity (IIRC) because electric pole prototypes do not support animations. Therefore, for the sprite to be visible in blueprints, the roboport component has to be blueprintable. In order for the electrical connections to also be preserved in blueprints, the electric pole component also has to be blueprintable. Both entities are associated with the "construction pylon" item, which is why that item gets double-counted.

Which means your theory was correct. But it is also kinda a known bug, because one of the ghost entities does not have a collision mask, which means it can even be placed over other entities with a blueprint.

KeinNiemand commented 1 year ago

Is there any reason why both entity need to be associated with the construction pylon item? I think that replacing the item of the entity with no collision to some kind of dummy item would prevent the no collsion entity beeing build when the main one can't be build yet thus fixing this issue this while still allowing both the be in the BP. I might try doing that at some point, you're free to try doing that yourself and submit a PR if you want it done faster, might be a while until I do it on my own. But like I said this is probably something that should be done on the SE side, it's changing something about how SE works under the hood and dosn't really have anything to do with my mod directly, not sure if the SE dev would make such a change just to be compatible with my mod tough so I'll probably have to do it here.

ChaosSaber commented 1 year ago

Sorry for not answering so long. Not using github much if I'm not working on something + long christmas vacation. Issue is fixed now with Space Exploration Version 0.6.95. The pylon have only one item in a blueprint.

ChaosSaber commented 1 year ago

I may have been a bit hasty with that. All the Ghost on Water pylons don't have any hitbox. They can be placed everywhere even over other buildings. They also don't mark trees/rocks for deconstruction. When SE still had two entities for each pylon in Blueprints they had a similar problem, that one of the entities could be blueprinted anywhere. Maybe the ghost on water one is based of the wrong entitity. Not sure. Does not happen with the original blueprint.

ChaosSaber commented 1 year ago

Looking through the in game debug info, I can see that every entity which can directly be placed into space has the "floor-layer" collision mask. The ghost on water dummies are missing them. Don't have much more time today to experiment with it or look up what the floor-layer actually means in factorio, but guessing from the vanilla entities which have this layer, they are entities you can walk on and that doesn't make much sense for power poles. So maybe one of the many Space Exploration hacks.

ChaosSaber commented 1 year ago

The pylons have the following collision layers: "floor-layer", "water-tile", "player-layer".
"water-tile" and "player-layer" are getting removed because it is colliding with water tiles. "floor-layer" is removed because it is colliding with shallow water from alien biomes. Which leaves the dummy with no collision mask.
The only solution I can think of is adding a custom collision layer to the dummies and all non tile entities.

KeinNiemand commented 1 year ago

I've already got a system for adding custom layers automatically it's what those special removals are for, (currently only used for object layer when SE is active), any layer that's in the special removals table get's removed from the water ghosts, and then get's replaced by a newly added custom layer, that custom layer is then also added to every entity that originally collied with the removed layer, It doesn't get added to tiles to it shouldn't cause water collisions. So adding one of those layer (maybe floor-layer) to special removals should fix this. Issue #30 and the problem where items on grounds don't get removed should be fixable the same way, just have to add the right layers to special removals. Just don't want to add to many things to special removals when I don't have to since that generates a new custom layer every time and I think there is a limited on how many layers can be in the game total.

ChaosSaber commented 1 year ago

That's good to know, that there is already a feature for this. Thankfully i got that info before investing more time into it. According to the wiki there are 55 layers, from which atleast 12 are reserved by the base game. Therefore i don't think adding up to 2 layers should be that problematic. I will try adding the floor-layer to it and report back.

ChaosSaber commented 1 year ago

Sadly, only works partially. I can no longer places the ghost over already build buildings of the same type, which makes tileable blueprints less painfull, but trees/rocks are still not marked for deconstruction as they don't have the floor-layer as collision mask.
Which means that the dummy layer needs to be added to these, even if they didn't have one of the special removal layers.

ChaosSaber commented 1 year ago

On the other hand, instead of using the floor-layer for this, it would probably be wiser to use the item layer. That would then also fix the issue with items on ground not being picked up.
And simultaneously open a bug report in SE to add the item-layer to all the buildings.

ChaosSaber commented 1 year ago

Seems like my previous tests where insufficient. The object-layer collision mask is not removed and therefore completely breaks the SE compatibility for empty space.

ChaosSaber commented 1 year ago

Fixed it i guess. Lua tables are complicated.

KeinNiemand commented 1 year ago

Still dosn't seem to work becouse item layer dosn't collide with the pylons in the first place, and apprently after testing making item layer collide won't mark items for deconstruction (even for regular BP/ghosts), so no need to even use item-layer. More on that on my comment on the PR.