Wrench rewrite #247
Wrench rewrite #247
Conversation
OgelGames
added
Enhancement
This comment has been minimized.
This comment has been minimized.
|
Another thing to think about: using original nodes with meta will add little overhead to all node placements. With most things this probably does not really matter other than having few relatively expensive calls but probably most people wont be placing a lot of wrench enabled nodes around or doing that with mesecons enabled devices while using mesecons_debug. Those are corner cases that actually can become bit worse but biggest thing is locking it without any possible way to revert that node override. Something to think a bit first, I'd like to get bit more opinions specifically about this feature lock. |
This comment has been minimized.
This comment has been minimized.
I don't think it is, digtron crates and inventories already have the same functionality (carrying lots of items), and are even more powerful because there is no hard limit on how much can be put inside them. (and yes I know, that should be fixed too)
To be exact, it was because it allowed nesting of chests (see the comment added in 0cf4133), which has now been fixed. The recipe could be made more expensive though, it is very cheap currently. |
How do you switch to another system if overriding node callbacks does not seem good? I think you cannot change it after introduced once while doing it any other way will leave options. (other than disabling wrench completely)
I'm not sure what you're telling here, idea is bad but can be added because other mod that does it should be also fixed? |
I like the idea of using that in survival mode 🤔 its not like you have a real advantage with this, aside from the mobile inventory Pandorabox-related:
👍 |
I do think that inventory extensions like this are not good, you could add mod with some 20x bag capacity instead and it will not be as powerful as wrench, especially bad if added to existing games and servers without explicitly deciding to do so.
True that pandorabox survival mode is extremely easy survival, this is not situation with some other servers that do still have technic available. |
Well yes, it would be complicated to change to another system (like a single item that changes texture), but I see no way to avoid that. Overriding the callbacks isn't required though, it's just the most efficient way to do it. (the alternative is
Hmm, maybe, considering there's be no recipe for over 7 years... 🤔
I'm saying there has been a far more powerful item available in survival for a long time.
👍 |
Registering proxy item for wrench is way to do that.
I do not really see that as alternative, just bit worse way to do exactly same thing... (it still locks you with callback that has to handle registered items and no way out) |
|
It seems that server crashes if large container with items that contain a lot of metadata is dropped, can probably be prevented by adding more items to blacklist (any item picked up by wrench that can cause large metadata like signs). Actual error is I see crafting configuration was made again be required so this probably is not that big problem as it is possible to restrict wrench use to trusted users. (after all wrench already had similar problems) |
Could also do what digtron crate does, preventing dropping completely: https://github.com/minetest-mods/digtron/blob/master/nodes/node_crate.lua#L386 Or check the size of the metadata and prevent dropping if it's too big. |
You can still drop digtron crates... luckily many players have not sent their larger trons through tubes but there's factory setups for that @ pandorabox. For node placement callback I would like to still get some opinions/comments from other people.It kind of is big thing because it is override that cannot be reverted afterwards, however bit easier now that crafting is not enabled by default as it decreases total amount of picked up nodes. |
I think this should probably be done when picking up node and prevent pickup if it would result in too large metadata. Dropping is still bad even if limited and preventing direct drop would still be good to reduce this. |
|
Related side note, I tried to hack up luanti-org/luanti#5686 and tested with I'm pretty sure someone with knowledge about engine itemdef/stack/inventory stuff could make it work in few minutes and also do proper complete implementation pretty fast. That would allow using proxy stack for wrench and some hope to get better UX in future. However what I've seen happening around engine pull requests I'm pretty sure it has to be 1. probably just someone else 2. probably someone with engine knowledge 3. someone who can implement it fast enough to not waste too much time when pr is left unnoticed or rejected. 4. someone with a lot of extra time to debate / fix style / test / doc requests for PR. |
|
There's another problem with that option: it would require the Also, whichever system we change to will have the same problem of being hard to change again (or revert), because there will no longer be separate items to alias. |
Click for detailed source code test coverage reportTest coverage report for Technic CNC 79.01% in 10/14 files:Test coverage report for technic chests 45.24% in 6/6 files:Test coverage report for technic 65.26% in 111/111 files:Raw test runner output for geeks:CNC: Chests: Technic: |
That simply is not true, feature is fully backwards compatible and items will work completely fine with old clients.
It is not hard but close to impossible to remove additional (and most of time useless) processing from node callbacks of other nodes and other mods. edit. @OgelGames If confusion is about possibly having stack textures in future then that is simply client enhancement and it does not affect usability in any way. |
|
Well, maybe I'm trying to protect other mods and special use cases bit too much here. Actually to update wrench again to use single proxy item with good user experience and without affecting or overriding other mods is just destroying all items inside picked up containers and maybe for that it is enough to have note in changelog. I guess admin should read changelog anyway, inform players and player who did not place their picked up containers in time will simply lose all items inside for not following advice. (still other opinions about this would be nice to have, I just see doing overrides as bad practice in general especially if causes troubles with future updates and in this case overrides could be completely avoided easily) |
This is actually quite elegant in terms of up- and downgrading: An picked-up chest with the new method just becomes a regular chest if you downgrade, nothing to clean up afterwards. IMO: i don't see an issue with it, there have been far worse hacks than this (not that i would describe this as a hack though) |
Improves and fixes the wrench, making it suitable for all players.
after_place_node)Also added support for digtron inventory nodes, and blacklisted loaded digtron crates.