robot256 / Vehicle-Wagon-2

Vehicle Wagon 2
MIT License
6 stars 2 forks source link

Pls add support for Turret Pod mod #13

Closed garrotte13 closed 1 year ago

garrotte13 commented 1 year ago

if remote.interfaces["zd-turretpod"] then remote.call("zd-turretpod", "unload_vehicle", vehicle, true) end in loadVehicleWagon.lua

and if remote.interfaces["zd-turretpod"] then remote.call("zd-turretpod", "reload_vehicle", vehicle) end in unloadVehicleWagon.lua

garrotte13 commented 1 year ago

Interface description is here: https://github.com/garrotte13/TurretPod/blob/396d491f648f27f6f84b32f8fc852cc8bcede735/scripts/reloadPods.lua#L515

robot256 commented 1 year ago

Hey, neat mod! I'm happy to work on compatibility. But I wonder if a special remote interface is needed at all. When vehicles are loaded, I raise the event "script_raised_destroy" with a custom parameter, "event.vehicle_loaded=true", so you can tell it from a normal destruction event. When vehicles are unloaded, I raise "script_raised_built" with the custom parameter "event.vehicle_unloaded=true". Can you use those events to register and unregister your pods?

It looks like you don't handle on_entity_cloned (used by Space Exploration spaceship launches), script_raised_built or script_raised_destroy (used by many mods for many reasons). You can fix many compatibility issues just by handling all of them as though the player had built/mined the vehicle.

garrotte13 commented 1 year ago

I tried without interface, but it didn't work out - I need to store grid before Vehicle Wagon destroys vehicle entity. Then it's too late. There's only one case when factorio doesn't kill grid for entity - it's character armor. All vehicle grids are destroyed by game engine when vehicle entity is destroyed.

https://lua-api.factorio.com/latest/events.html#script_raised_destroy quote: "The entity that was destroyed."

Unfortunately factorio API has a lot of events which inform about something that has already happened and it's too late to catch and save something. People ask them about changing order, but Wube rejects it (most often and worst factorio API example - player is taking out equipment from grid)

garrotte13 commented 1 year ago

It looks like you don't handle on_entity_cloned (used by Space Exploration spaceship launches),

Thanks for info. I'll put it in to-do list. I don't play with SE. It's beautiful, but breaks game balance completely.

robot256 commented 1 year ago

I tried without interface, but it didn't work out - I need to store grid before Vehicle Wagon destroys vehicle entity. Then it's too late.

script_raised_destroy should never be raised for an invalid entity unless some other mod deleted it prematurely in their own event handler. Not sure if the default behavior deletes the grid but not the entity before raising the event? But my code raises the event manually before any destruction actually occurs:

  script.raise_script_destroy{entity=vehicle, vehicle_loaded=true}
  vehicle.destroy()

The docs are misleading because events like that always contain "The entity that is being destroyed". There would be no purpose to the event if it said "something was destroyed but you can't find out what anymore".

If the event is raised with a vehicle before its destruction but after the grid is invalid/unlinked, then you could still reference your saved status data based on the vehicle's unit_number.

Edit: I tested it just now, and the default script_raised_destroy behavior does indeed provide a valid LuaEntity with a valid LuaEquipmentGrid attached. Normally, the problem is when mods call "entity.destroy()", which does not raise the event at all, instead of "entity.destroy({raise_destroy=true})", which does raise the event. That is their fault, not yours.

I don't play with SE. It's beautiful, but breaks game balance completely.

SE is pretty well balanced for a work in progress, it's just completely different from the vanilla game.

In order to handle SE spaceship launches, you need to treat "on_entity_cloned" as a build event for the "destination" entity. But cloning itself doesn't automatically delete the "source" entity, so SE raises a separate "script_raised_destroy" event for the source entities that are deleted after cloning. There is talk of changing this for vehicles that can be teleported, but it would be good to make it work in either case.

garrotte13 commented 1 year ago

Hey! I encountered a critical issue in Building Time vs Vehicle Wagon 2 compatibility and in the end found it was caused by raise_script functions you love so much. Easy fix. Looks like even Klonan isn't aware of your "raise event" philosophy. ;-) Probably should push all features and issues of Building Time to Klonan if he agrees. By the way, BT is one of the greatest factorio mods (together with Rampant those two make Factorio a true game).

Anyway that issue with BT vs VW2 reminded me about your advise how to fix TurretPod vs VW2.

The docs are misleading because events like that always contain "The entity that is being destroyed".

Ok, I'll try to change my code the way you suggest. Will let you know how it goes. I'm a gamer first, and far less a fan of mod coding. Still playing and testing previously implemented features in mods.

In order to handle SE spaceship launches...

After finding a number of dumb things in Earendel mods (recent one was in AlienBiome... why should someone change logics for entire game when adding some tiles beauty only...) I do try to keep distance from all his mods. And recommend it to everyone.

robot256 commented 1 year ago

In order to handle SE spaceship launches... I do try to keep distance from all his mods.

Fair enough, but cloning happens in other contexts as well. Most notably in editor mode, and various "testing lab" mods. It's part of the API, so it's better if you can support it.

garrotte13 commented 1 year ago

Hi! I spent some time and changed code the way you suggested and it works like you predicted, BUT... there is a mistake in your logics - you make a backup copy of inventory before triggerring script_raised_destroy, therefore inventory change made by TurretPod mod is wasted by Vehicle Wagon :-(

garrotte13 commented 1 year ago

This line https://github.com/robot256/Vehicle-Wagon-2/blob/5281988669a09a2183f596d9a240a679bc98940b/script/loadVehicleWagon.lua#L141 must be before this block https://github.com/robot256/Vehicle-Wagon-2/blob/5281988669a09a2183f596d9a240a679bc98940b/script/loadVehicleWagon.lua#L49

robot256 commented 1 year ago

Ordinarily script_raised_destroy is raised by the destroy() function itself. Making changes to the entity that is being destroyed isn't really a supported operation IMO. Why do you need to make changes to the equipment before I save it? Can't you restore the state in your mod if I restore the equipment as it existed already?

garrotte13 commented 1 year ago

Ordinarily script_raised_destroy is raised by the destroy() function itself.

I moved the event trigger in your mod code and it worked fine. Do you mean that other mods may use instead a raise event flag in destroy function? I don't know. Then your advise was wrong and I wasted time on code change....

Why do you need to make changes to the equipment before I save it? Can't you restore the state in your mod if I restore the equipment as it existed already?

I need to store data from equipment grid as ammo in inventory.

Technically it is possible to move this data to special data array assuming, that killed grid will revive when vehicle will be unloaded by your mod at some moment. The stupid thing is Factorio killing grid. Why don't they kill char armor grid, but kill vehicle's grids - illogical decision...

robot256 commented 1 year ago

Then your advise was wrong and I wasted time on code change....

My earlier comment was made when I thought you simply had to identify which grid to read, like some of the Electric Train mods. I did not realize there was information stored outside the equipment grid that your mod needs to save and restore.

I need to store data from equipment grid as ammo in inventory.

This sounds like a workaround, and we can implement a better solution (that won't be affected by items being mixed up or inventories being full). For example, VW2 receives a special data table from Autodrive and Car Keys. Every time VW2 tries to load a vehicle, it asks Autodrive and Car Keys if they have any data they need to save, and saves it in the global table. Then when the vehicle is unloaded, after the vehicle entity is created, tells Autodrive and Car Keys what data had been saved so they can re-apply it.

Can you make two interface functions that do the following:

  1. saveTurretPod.Data: takes the vehicle entity being saved as an argument, returns a Lua table with the information you need to eventually restore it, or nil if there is no data needed.
  2. restoreTurretPodData: takes the restored vehicle entity and the associated saved Lua table as arguments, and applies the data to the new vehicle.
robot256 commented 1 year ago

To summarize, if you make an interface that returns a table that contains the name and amount of ammo to put in each turret when they are revived, then you don't need to actually put anything in the inventory. You can recreate the ammo by script when the restore function is called. VW2 will call the interface before it tries to destroy the vehicle.

Then your handler for script_raised_destroy does not have to worry about saving anything. It only has to delete the reference to the grid and any companion entities that were associated with it.

garrotte13 commented 1 year ago

Hi, Some player came with incompatibility problem vs helicopter mod. I'll look through it and maybe I need some additional work around code to store data in array - it will solve issue for all. Or maybe it is some on_entity_cloned feature you warned me about. Or maybe that mod doesn't use script_raise events...

garrotte13 commented 1 year ago

oh, that crazy mod changes entity of vehicle every few seconds and transfers inventory+grid equipment every time. Without any raisescript triggers. I'll think about different approach - don't do anything with equipment and inventory when some other mod is transferring them. Technically the only problem is to get knowledge, where it goes. TurretPod already knows that vehicle was not killed or mined.

garrotte13 commented 1 year ago

I can modify TurretPod to correctly process vehicles on entity destruction even without a destroy event raised. I can handle created vehicles with create event raised like your mod does. But I need some logics to understand when new equipment grid is a revival of previously destroyed one (or equipment inside it). I'll do some tests, maybe factorio engine keeps some objects. If VW2 restores equipment with the same energy stored values as they were when loading vehicle then I can easily solve it - I'll add some more logics to not nullify energy and weapon state. That would be real great - unloaded tank will be ready to shoot immediately after deployment on the ground :-) That's even better than exchange info between mods.

robot256 commented 1 year ago

Yes, VW should save and restore every readable property of the equipment including energy and shield value. It also saves burner energy source fuel items if they exist. If you find anything unusual with that process let me know!

garrotte13 commented 1 year ago

Tested - works fine!

garrotte13 commented 1 year ago

Wow, in Factorio 1.1.85 game engine doesn't destruct vehicle equipment grid. It works like with char armor! And they didn't post about it anywere.

robot256 commented 1 year ago

Wow, in Factorio 1.1.85 game engine doesn't destruct vehicle equipment grid. It works like with char armor! And they didn't post about it anywere.

That worries me a little, what context is that in? The grid probably persists if the vehicle is mined into an item because it gets attached to the item. Or for the duration of the destroyed event. But if the vehicle is destroyed, shouldn't the grid be destroyed eventually no matter what? What can you actually do with a grid object that isn't attached to a valid vehicle anymore? Otherwise you could get a memory leak where lots and lots of grids exist that can never be re-attached to any vehicle.

garrotte13 commented 1 year ago

Grid was being destroyed for vehicles when mining them. Now it is alive and available. Wube team fixed it. So now all vehicles behave like character armor - inventory access is lost, but grid is kept (but not functioning - solar cells and portable reactor don't charge batteries) I don't know when it was fixed. Last winter it didn't work this way and I had to write a lot of code. Now half of logics for it is redundant and I'm refactoring this part...

robot256 commented 1 year ago

Very interesting. I know that the grid information was always preserved as an item, and displayed in the tooltip of the item, but I guess it was not accessible through the API in that state. Glad to hear it has improved.

I assume the grid is still destroyed when Vehicle Wagon stores the vehicle and destroys it? And you encoded your information in the grid characteristics so they are saved successfully? Let me know if there is anything else you need.

garrotte13 commented 1 year ago

I know that the grid information was always preserved as an item, and displayed in the tooltip of the item, but I guess it was not accessible through the API in that state. Glad to hear it has improved.

Vehicle grid was not available that way for players and it was being destroyed for API. Then re-created when player deploys that vehicle again.

I assume the grid is still destroyed when Vehicle Wagon stores the vehicle and destroys it?

I assume it does, but it is not important for me after TurretPod mod changes anymore. My mod doesn't create grids or other entities. But it creates elements in mod global table variables. And I made sure, that those elements are destructed when raise_script_destroy for vehicle is run. Also I added trash clean logics if some poorly coded mod doesn't run this method/flag. So on my side everything is great in all cases. I played with helicopterrevival mod a little and added required events there like you have in VW2 to make it work correctly with Turret Pod. Helicopter is really beautiful and sounds awesome (even better than oldschool Desert Strike). I understand now why common people get bedazzled by nice-looking stupid balance-breaking mods like the ones from Earndel.