search

pandorabox-io / pandorabox.io

Pandorabox infrastructure code
https://pandorabox.io
31 stars 4 forks source link

digistuff channel copier is very limited #550

Open SwissalpS opened 4 years ago

SwissalpS commented 4 years ago

The current code states that other mods should add some field to be compatible. I think that is not a nice way to go about it.

I have forked it and added some more nodes. Needs to be tested and then either we make an override or maintain code ourselves, as it does not look like upstream agrees with this approach.

@S-S-X @OgelGames @6r1d

BuckarooBanzay commented 4 years ago

as it does not look like upstream agrees with this approach.

Did you bring that up to cheapie? Are you sure this does not work out somehow? :confused:

I saw your PR on the mirror repo: https://github.com/minetest-mirrors/digistuff/pull/1

This is one of those brilliant free software situations where a new fork is created, is it? :smirk: How can we still get future maintenance- and feature-updates?

At first glance it looks like this can be done with overrides too (the description and node-definition fields) wouldn't that be a viable option too?

Anyway, i'm neutral to this and can live with either decision (i do have a few enhancements though if we would fork it...)

SwissalpS commented 4 years ago

Yes, I think we can easily fix situation with an override. Take the list from my fork and process it in a pandora_custom script.

S-S-X commented 4 years ago

i do have a few enhancements though if we would fork it...

Implementing #183 would also become trivial task, also cleanup of stuff added for #539

OgelGames commented 4 years ago

IMO forking would be better because there are other things to fix, like what @S-S-X mentioned, and also the game controller, which currently doesn't work correctly because of this: https://github.com/minetest-mirrors/digistuff/blob/c4b40bcfd5c7116a48af87e3db07a9c29757fed9/controller.lua#L96

SwissalpS commented 4 years ago

for now, I propose this fix Tests will show if I got some entries wrong. Also they will show which nodes need to have an update hook added.

SwissalpS commented 4 years ago

have tested and updated. Only force field and supply converter don't work properly yet. Fancy vendor has it's own tool, so I didn't bother with that.

Edit: sorry for the merge-mess. Easier to just read the final file.

SwissalpS commented 4 years ago

Thanks @S-S-X your link saved me some lookups and now force field and supply converter also work.