turtle.suck(0) returns true all the time.

cc-tweaked / CC-Tweaked

Just another ComputerCraft fork

https://tweaked.cc

883 stars 207 forks source link

turtle.suck(0) returns true all the time. #824

Open SkyTheCodeMaster opened 3 years ago

SkyTheCodeMaster commented 3 years ago

Useful information to include:

Minecraft version: 1.16.5
CC: Tweaked version: 1.96.0
Logs: These will be located in the logs/ directory of your Minecraft instance. Please upload them as a gist or directly into this editor. (Literally nothing in the logs)
Detailed reproduction steps: sometimes I can spot a bug pretty easily, but often it's much more obscure. The more information I have to help reproduce it, the quicker it'll get fixed. Place a turtle, run turtle.suck(0), regardless of whether there is an item in front, it will return true.

https://github.com/SquidDev-CC/CC-Tweaked/blob/d71bf225cc5e4d9f1a2ca050982489bab3c366ea/src/main/java/dan200/computercraft/shared/turtle/core/TurtleSuckCommand.java#L43-L47 From discord: https://canary.discord.com/channels/477910221872824320/477911902152949771/853375167232999455

SquidDev commented 3 years ago

So technically this behaviour is working as intended. However, I think it's more useful if we change it.

Wojbie commented 3 years ago

I suggest to error in this case. Making it return false would kinda break backwards compatibility.

SquidDev commented 3 years ago

So would erroring though :p.

SkyTheCodeMaster commented 3 years ago

Besides, it's not like people would do if condition == turtle.suck(0) then in place of if condition [== true] then ... right?

fatboychummy commented 3 years ago

Could make turtle.suck(0) return true if items exist in whatever it's trying to pull from, false if empty; Much like turtle.refuel(0) can be used to determine if an item is usable as fuel.

SkyTheCodeMaster commented 3 years ago

Could make turtle.suck(0) return true if items exist in whatever it's trying to pull from, false if empty; Much like turtle.refuel(0) can be used to determine if an item is usable as fuel.

That's quite literally what this issue is about, chummy.

fatboychummy commented 3 years ago

Could make turtle.suck(0) return true if items exist in whatever it's trying to pull from, false if empty; Much like turtle.refuel(0) can be used to determine if an item is usable as fuel.

That's quite literally what this issue is about, chummy.

No, this issue is "turtle.suck(0) returns true all the time." Unless this was what you meant to have changed when you posted this? Probably should have included that in your bug report.

My proposition was to change that, but as woj has pointed out in MCCM it would be a bad "magic number implementation" and we probably shouldn't do so.

uecasm commented 2 years ago

The documentation states that if it fails to pick up items then it is supposed to return false, "reason". So it should actually be doing that instead.