Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add scanner node #540

Open
wants to merge 12 commits into
base: master
Choose a base branch
from
Open

Add scanner node #540

wants to merge 12 commits into from

Conversation

johalun
Copy link
Contributor

@johalun johalun commented Oct 8, 2020

The Scanner Node

The scanner node will scan any inventory of the node behind it and output a signal depending on its configuration. It also has digilines support (which means every node with inventory has suddenly become digiline aware, albeit in a limited manner).

Functions:

  • Placement direction same as delayer (input to the left, output to the right)
  • Right click to open formspec and configure options
  • Formspec
    • Dropdown list with choices of inventory to scan
    • Low/high watermark input fields which is percent of slots occupied by any number of items in target inventory
    • Presets for quick configuration
      • Empty: output on if empty
      • Has items: output on if at least 1 slot contain at least 1 item
      • Full: output on if all slots are occupied by at least 1 item
    • Digiline channel name input
    • Invert output option
    • Save button
  • Will send output status { output = "on"/"off" } on Digiline "GET" message
  • Will send output status on Digiline channel when output value changes
  • Timer fires every second to check inventory

I'm not sure about the timer though. Currently it checks once per second. Don't know what's appropriate here.

This is my first ever Minetest mod development and first contact with Lua as well so please be gentle :)

Screenshot from 2020-10-08 12-53-17
Screenshot from 2020-10-08 12-53-32
preview
recipe

@auouymous
Copy link
Contributor

I'll look at the mod and source later today or tomorrow.

The recipe should be more expensive, see detector. Maybe add a microcontroller and wire up the center and remove the torches.

You could allow the values to be configured via digilines, I think the detector allows that. The detector can also return which node it sees over digiline, perhaps this could return the exact slot count along with output status in its GET message.

@johalun
Copy link
Contributor Author

johalun commented Oct 8, 2020

Yeah, given that it's pretty powerful it can be more expensive. That's why I added the crystal but maybe that's not enough?

Digiline functionality can also be expanded. Have it be programmable via digilines was on my roadmap but I wanted to get the PR done in reasonable time. I'll take a look at the detector and maybe push some more digiline code to the branch.

@johalun
Copy link
Contributor Author

johalun commented Oct 8, 2020

Maybe also leave digiline channel field empty and only send messages in case it has been configured.

@auouymous
Copy link
Contributor

Place an inventory and scanner, click empty button and save. Place wire or lightstone on output... does it work for you?

@johalun
Copy link
Contributor Author

johalun commented Oct 8, 2020

Place an inventory and scanner, click empty button and save. Place wire or lightstone on output... does it work for you?

Nope, it doesn't power the the wire. Might have been introduced when I added the "only update on change" part.

@Desour
Copy link
Contributor

Desour commented Oct 8, 2020

Didn't test.
Looks pretty nice, from description, screenshots and code.
There are some little things that could be improved (eg. it would probably be better if you don't set the formspec meta field, but open the formspec in on_rightclick instead (easier for dynamic things and changes with versions + old clients don't fire on_rightclick if formspec is set via meta)).
((((However, I'm not sure if it makes sense adding such a node to the mesecons mod(pack). Mesecons currently doesn't do much stuff related to inventories.))))

@johalun
Copy link
Contributor Author

johalun commented Oct 8, 2020

Mesecons currently doesn't do much stuff related to inventories

Which is exactly why it's needed :)

@johalun
Copy link
Contributor Author

johalun commented Oct 8, 2020

New recipe

recipe

@johalun
Copy link
Contributor Author

johalun commented Oct 8, 2020

The scanner is now programmable and you can also get all stats via digiline GET message. On output change it still sends a short message with only the output state.

Screenshot from 2020-10-08 16-49-42

@johalun
Copy link
Contributor Author

johalun commented Oct 9, 2020

@auouymous I fixed the output update bug by always updating the mesecon state on timer callback and not only when output is changed. To avoid this, is there a callback that is called when adjacent nodes are updated/changed so we can just do the mesecon state update there instead of on every timeout?

Another issue is the wield image. If I set the image it just looks flat. If I don't set the image, it renders the node fine but it's too low on the screen and barely visible. Is there a wield_offset, like wield_scale? Edit: Or a wield_rotate might work also to show it on it's side.

@auouymous
Copy link
Contributor

The problem is that you set both nodes to the off state. Change the "on" node to the "on" state and it works fine. And then undo the fifth patch that always sets state.

- mesecons = { receptor = { state = mesecon.state.off } }
+ mesecons = { receptor = { state = mesecon.state.on } }

@johalun
Copy link
Contributor Author

johalun commented Oct 9, 2020

The problem is that you set both nodes to the off state. Change the "on" node to the "on" state and it works fine. And then undo the fifth patch that always sets state.

- mesecons = { receptor = { state = mesecon.state.off } }
+ mesecons = { receptor = { state = mesecon.state.on } }

If I do that the scanner will power adjecent nodes, also on the sides.

@auouymous
Copy link
Contributor

That is what output rules are for. Only the output side should be returned.

@johalun
Copy link
Contributor Author

johalun commented Oct 9, 2020

Ah I see now. I based this off the switch at first and then I realized it was more like the delayer and started looking at that code instead. I see now what I missed.

@auouymous
Copy link
Contributor

on_timer should recalculate low/high if i_size ~= meta:get_int("inventory_size"), otherwise a chest upgrade/downgrade will have incorrect low/high values.

Am I right to assume that a chest with 101 slots will have low=2 for "has items"?

@johalun
Copy link
Contributor Author

johalun commented Oct 9, 2020

Good catch on both points. Should work as intended now.

@auouymous
Copy link
Contributor

Sorry for the late reply, it seems to work as expected. Although, it would be nice if it could power from its two sides to allow more compact setups.

@johalun
Copy link
Contributor Author

johalun commented Oct 23, 2020

Although, it would be nice if it could power from its two sides to allow more compact setups.

After building with it a few times I was thinking the same thing. I will fix that.

Another thing that I'm not quite sure how to fix is how to make it look good when wielding. Currently it's just a flat texture. If I use the 3D model, it's way too low and barely visible. Is there a way to change the offset of how it is displayed when wielding?

@johalun
Copy link
Contributor Author

johalun commented Oct 23, 2020

I removed the wield image so that the 3d model is displayed in hand. I think that's better. Even though it's low, you can still see it. This problem can also be seen with insulated mesecon which isn't visible at all so I think it's ok.

@auouymous
Copy link
Contributor

Logic gates and luacontroller use their model and are hard to see as well.

@h3ndrik
Copy link

h3ndrik commented Nov 28, 2021

i'm no expert in this, but maybe this should be integrated into another mod like https://github.com/minetest-mods/MoreMesecons
if it doesn't get merged here?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants