Zomis / FactorioMods

Various mods for the game Factorio
https://mods.factorio.com/mods/zomis
MIT License
15 stars 9 forks source link

Allow copy paste to paste the product recipe on to first_signal of a decider or arithmetic combinator. #54

Closed FranOis closed 1 year ago

FranOis commented 1 year ago

When using a mojod style mall, you need to copy the recipe ingredients on a constant combinator which is already done with this mod, but you also need to copy the product to an arithmetic or decider combinator. This change allows the mod to do just that. It is off by default but can be turned on if the user wants it. It will take the first product that is outputted by the recipe and will set it as the first_signal on either an arithmetic or decider combination. None of the other parameters on the combinator will be changed, this allows the user to set whatever logic they want in their blank blueprint and only have to change the assembler's recipe and copy paste on all combinators that requires it for the mall to work as intended.

Zomis commented 1 year ago

Hi! Thanks a lot for the pull-request!

I'm not sure it makes any sense to have settings for allowing to copy to arithmetic/decider combinators. If the user does paste to arithmetic/decider combinator and the setting is off, then nothing would happen otherwise? So it makes sense to have it as default behavior to me. Or is there any time a player would paste something to an arithmetic/decider combinator and not have this functionality?

(I don't have time right now to merge and release a new mod, but I will do so during the weekend!)

FranOis commented 1 year ago

I added the settings to make it less disruptive, but I can remove them, was thinking maybe someone had another mod that did something to those combinators and now the behaviour is disrupted with this update, might be over engineering. To be honest, I wanted to know also how easy it was to add a setting, as this is my first time coding part of a factorio mod. Would you prefer I make it true by default or remove the settings all together?

No rush on publishing.

Zomis commented 1 year ago

I think it is better to have it as opt-out, so please change the default setting to true. I added some other comments also in the pull request that I would appreciate if you would fix :)

FranOis commented 1 year ago

I ammended my previous commit so that the history stays cleaner. I've addressed your comments, let me know if you prefer the if elseif format. I'll be playing with this update tonight so I shall test it out :)

FranOis commented 1 year ago

I'm not sure why jenkins is failing since I can't login, but the changes I've made are working on my factorio copy. Let me know why it's failing and i can fix it.

Edit: looks like it finally passed :D

Zomis commented 1 year ago

Now published in version 1.3.0: https://mods.factorio.com/mod/copy-paste-recipe-signals/downloads

Many thanks!

FranOis commented 1 year ago

Awesome, thank you :)