Xcone / factorio_pump

MIT License
1 stars 5 forks source link

Compatibility for Factorio-Tiberium mod #14

Closed Ferlonas closed 1 year ago

Ferlonas commented 1 year ago

https://mods.factorio.com/mod/Factorio-Tiberium https://github.com/James-Fire/Factorio-Tiberium

As I mentioned in the mod page, P.U.M.P. can unfortunately not be used for automated placement of tiberium spikes because the resource they can be placed on is not a liquid before placement. tiberium-pump-compatibility.zip

Xcone commented 1 year ago

Thanks for the save.

Early look suggest P.U.M.P. should be able to pick up on it: image

It definitly has found a liquid resource form the tiberium mod, and figured out it needs to put a tiberium spike on it when selected.

So it seems to be a selection issue, and not a resource type issue. I'll look a little deeper and get back to you later.

Xcone commented 1 year ago

Right, .. you weren't kidding. They replace the actual resource upon placement of the Spike. I was actually assuming/expecting some resource mapping was missing somewhere. But it all looks good.

Specifically, the problem is the surface has a tibGrowthNode, which has the advanced-solid-tiberium resource. The spike, which can do both advanced-solid-tiberium and advanced-liquid-tiberium, .. the solid not-reallym though. But, the placement of the spike, replaces the tibGrowthNode with a tibGrowthNode_Infinite, which has the advanced-liquid-tiberium as result, which the spike can properly do. Therefor, P.U.M.P. properly picks up on the liquid bit, but that's completely useless, since the surface only has solids.

I don't really see a way out of that, unfortunately. I was thinking to maybe check each miner that can do a liquid resource, to also scan for its solid resources. But I fear that might cause yet other issues with other mods. Unfortunately it's the resource that dictates the mining result, and not the miner. So another mod could have a miner that can do both liquids and solids, and has outputs for both as well. With the change, P.U.M.P. would draw wrong conclusions on that.

I'm also not a fan of adding mod-specific exceptions to P.U.M.P. I've successfully avoided it so far and would prefer to keep it that way. Perhaps the folk of the tiberium mod can properly add the fluid resource on the surface, instead? Or add both resources on the surface on the same spot, and remove the one, or the other, depending which miner is placed?

Ferlonas commented 1 year ago

I'll talk to them and see if there's a way around it. However, I found a possible workaround in the meantime, which unfortunately crashes. With normal pumpjacks, after I have placed them, I can use P.U.M.P. afterwards to just link them. This should work with the tiberium spikes as well, however, it will crash for some reason.

Error while running event pump::on_player_selected_area (ID 51)
__pump__/toolbox.lua:290: attempt to index field '?' (a nil value)
stack traceback:
    __pump__/toolbox.lua:290: in function 'add_available_extractors'
    __pump__/toolbox.lua:335: in function 'add_toolbox'
    __pump__/control.lua:60: in function 'process_selected_area_with_this_mod'
    __pump__/control.lua:8: in function <__pump__/control.lua:6>

Is that an unforeseen circumstance in your mod, or is that due to an implementation error on the tiberium spike?

Ferlonas commented 1 year ago

It seems to have issues when accessing the property fluidbox_prototypes on the tiberium spike. I wanted to see what the issue could be, but I found no documentation on this property anywhere in the base game files. Can you shed more light on it?

Xcone commented 1 year ago

"With normal pumpjacks, after I have placed them, I can use P.U.M.P. afterwards to just link them. " You can? I didn't know that. Did you make that yourself, or does it come out of the box? It's not something I intentionally added for sure. I would expect the linking to fail because P.U.M.P. doesn't get to chose the rotation of the pump.

_"It seems to have issues when accessing the property fluidboxprototypes on the tiberium spike." A lot of data is lacking. Much of the API I found is merely the "these things exists" and "this is how they interact". But what's actually there is a mystery, at least in the API docs.

I recommend checking out this repository. It's the prototype definitions of the core game and is very insightful: https://github.com/wube/factorio-data

It's not giving the exact answers as well. But you'll find that a pumpjack has 1 output fluidbox image

A fluidbox is basically "something that hold fluids". And you specify how the pipes can access it with the "pipe_connections" property.

You'd be tempted to think P.U.M.P. should read output_fluid_box instead of fluidbox_prototype. But there's a difference between the prototype (like the repo), and how they end up in the game entities after all prototypes are loaded.

I'm pretty sure the spike does have one. See, when I was doing my own checks, I did add a temporary hardcoded exception in P.U.M.P. to allow spikes to be placed on the tibGrowthNode. There was one other fix I had to do, but then PUMP worked on tiberium as you requested. It, unfortunately, is not a sustainable solution as explained earlier. But it does prove the fluidbox_prototype exists on the Spike. So if you encounter an issue that it suddenly doesn't you might want to check the stage where you try to access it. There's several loading stages of a mod, and not all stages have access to the same kind of info.

Perhaps you can shed some more light on type of changes you've made so far?

Xcone commented 1 year ago

In terms of what I can do from P.U.M.P. while still ignoring other mods exist ...

As I posted, I had to change 2 things.

  1. Change the selection-tool to accept selection of tibGrowthNode
  2. Further restrict which miners can be used, by checking if the available miners support fluids at all.

I don't really mind adding the 2nd thing. It's a restriction I don't really need in a regular scenario, but it doesn't hurt in terms of knowing which specific other mods are out there.

The 1st thing I have issue with, because I need to make an exception just for the tiberium mod. But ... I think mods can make alterations to other mods. I think you might be able to make a mod, that alters the selection-tool of PUMP, to also add tibGrowthNode to the selection filter.

image

image

I think a mod that executes these 2 lines in the data-final-fixes should do the trick:

table.insert(data.raw["selection-tool"]["pump-selection-tool"].entity_filters, "tibGrowthNode");
table.insert(data.raw["selection-tool"]["pump-selection-tool"].alt_entity_filters, "tibGrowthNode");
Xcone commented 1 year ago

You could even ask the folk of the Tiberium mod to add these. They seem to have mod specific exceptions in place already.

Do tell if you wish to persue this. Then I'll make the other change I needed to do. It's not something that can be done from another mod.

I also understand if you want to finish what you started right now.

Xcone commented 1 year ago

I just uploaded version 1.1.4. of P.U.M.P. with the above change in it. I realized it was a good change to add. Because P.U.M.P. shouldn't attempt to work with the miner's inputs anyway. So I was able to make it a little more secure in case yet another ever works with an input AND output pipe. And at the same time, the 2 lines of code for final fixes should now be enough to get it to work with tiberium, too.

I hope this helps. I'm curious to hear if you end up making your own small mod, or if it get's embedded into Tiberium. Keep me posted :-)

Ferlonas commented 1 year ago

"I would expect the linking to fail because P.U.M.P. doesn't get to chose the rotation of the pump." - Turns out, what it actually did was deconstruct the existing extractor and place a ghost for a new one on top of it, with the proper orientation. No change was necessary to enable that, but it still provokes that crash. As spikes were placed on the nodes, the nodes had fluid resources. P.U.M.P. realized that and also seemed to grasp that there was already an extractor on it. Then it tried to connect those extractors, and crashed with the aforementioned stack trace. Trying to trace it back, I just added some temporary variables, so instead of

  local cardinal_output_positions =
    extractor.fluidbox_prototypes[1].pipe_connections[1].positions

I had

  local tmp1 = extractor.fluidbox_prototypes
  local tmp2 = tmp1[1]
  local tmp3 = tmp2.pipe_connections
  local tmp4 = tmp3[1]
  local cardinal_output_positions =
    tmp4.positions

With the crash now resulting in the assignment to tmp2, factorio claiming it couldn't access the properties of tmp2, as it was nil.

As a sidenote on this: I also followed your other suggestion and added both the fluid and the solid resources to the solid node, with the following reasoning: If P.U.M.P. sees that there is a fluid resource, it will construct ghosts with the appropriate extractor, as it usually does. Once that extractor is placed, the solid node will be replaced with the usual fluid node, and the spike will work as it should. However, it resulted in exactly the same stacktrace, which is why I thought it was the extractor's fault, as this works with oil pumps.

I just made some adjustments to their mod and tested your change, and it works perfectly. Thank you so much for that simple solution. I will make a pull request in their repo and ask them to put it in for the next version.

I also have a small pull request for you that I'll open once I've tested those changes. It's related to another topic though.

P.S.: Thank you very much for the insight into the factorio API, and for your changes

Xcone commented 1 year ago

Sounds like we came to an agreeable solution for this issue. I think we can close this one now. :-)