Baritone AI pathfinder

Baritone AI pathfinder

72.7k Downloads

buildIgnoreProperties does not work as expected

mvrozanti opened this issue ยท 12 comments

commented

Some information

Operating system: GNU/Linux 6.2.12-arch1-1
Java version:
openjdk version "17.0.7" 2023-04-18
OpenJDK Runtime Environment (build 17.0.7+7)
OpenJDK 64-Bit Server VM (build 17.0.7+7, mixed mode)
Minecraft version: 1.19.4
Baritone version: I'm not sure, but I think it's 1.9.2, since Meteor sets it as dev build and #version command outputs it as null. But I believe it doesn't matter, it should be one of the latest, that includes the recently added buildIgnoreProperties setting.
Other mods (if used): fabric, meteor

How to reproduce

  1. Do #set buildIgnoreProperties axis

  2. Try to #build a schematic containing some jungle_logs with properties axis=x, some with axis=y and some with axis=z

  3. Wait for baritone to stop and output "Missing materials for at least: 1x Block{minecraft:jungle_log}[axis=x]" or a similar message instead of continuing, which is the expected behavior

Modified settings

buildIgnoreBlocks (List<class_2248>)
buildIgnoreDirection (Boolean)
buildIgnoreExisting (Boolean)
buildIgnoreProperties (List<String>)
buildInLayers (Boolean)
buildSkipBlocks (List<class_2248>)
startAtLayer (Integer)

Final checklist

  • I know how to properly use check boxes
  • I have included the version of Minecraft I'm running, baritone's version and forge mods (if used).
  • I have included logs, exceptions and / or steps to reproduce the issue.
  • I have not used any OwO's or UwU's in this issue.
commented

Is it standing at the position where it would have to place a log (i.e. standing in it's own way)? Maybe even at the only nearby position that's still wrong?

commented

I'm not sure I understand what you mean. After it logs the missing materials message, if I move the player around it still won't continue, even after a #resume call, if that answers your question. So I don't think it is standing in it's own way or stopping because it is occupying the position of the block that should be placed.

I was reading through the PR that adds this setting and I couldn't understand what are these three conditions that trigger sameBlockstate to return false. This is a wild guess but is it correctly comparing properties?

commented

So I don't think it is standing in it's own way or stopping because it is occupying the position of the block that should be placed.

That's exactly what I was asking for. In a quick test I got it to fail like that.

I was reading through the PR that adds this setting and I couldn't understand what are these three conditions that trigger sameBlockstate to return false. This is a wild guess but is it correctly comparing properties?

The logic is a bit mindbending indeed. Probably should've changed the structure from a single if to multiple.
The reasoning is that

  1. (this should be 0. but I can't get GitHub to do that) The blocks might be different, in which case we bail out so for the rest of the method we can assume that both states have the exact same list of properties. (it also speeds things up)
  2. To return false there has to be a property with mismatched value. Property values are singletons so we compare with !=.
  3. That property may not be ignored due to buildIgnoreDirection.
  4. The property must also not be ignored by buildIgnoreProperties. The setting is a list of names so we check the name instead of the property itself.
commented

Slightly reordered code with comments

    for (IProperty<?> prop : map1.keySet()) { // can iterate over only one key set because all states of a block have the same set of properties
        if (map1.get(prop) == map2.get(prop)) {
            continue; // no difference if the values are the same
        }
        if (ignoreDirection && orientationProps.contains(prop)) {
            continue; // pretend that there's no difference if the property is ignored
        }
        if (ignoredProps.contains(prop.getName())) {
            continue; // pretend that there's no difference if the property is ignored
        }
        return false; // value mismatch and it's not ignored, the states can't be considered equal
    }
    return true; // no mismatch we care about, first and second are equal for us
commented

So if that part is correct, what could be causing the setting to not take effect?

commented

I was about to ask you to print out approxPlaceable when it fails (I'm still interested in it just to be sure) and while looking at the code in an attempt to back my guessing with facts I noticed that this check checks for complete equality between the desired state and states in approxPlaceable even though buildIgnoreProperties and buildIgnoreDirection should make it check for partial equality using sameBlockState. This prevents the respective position from being added to placeable and hence Baritone won't try walking there, unless there's another reason to do so. If this is indeed the cause I'd expect it to fail whenever there are no logs to be placed nearby but some logs are still missing (there should also be connections to other incorrect blocks but they are much less clear because the can depend on basically anything between running the command and failure).

commented

Is this approxPlaceable data printable without forking Baritone? As in, via a command or by enabling a setting? Or do I have to get my hands dirty in the code?

commented

Not available without code editing. I have managed to reproduce this so I can also do it myself now.

Steps to reproduce (tested on master / 5f2eadb):

  1. Create a 1x1 selection (#sel pos1 and #sel pos2)
  2. Place a horizontal log inside the selection
  3. Copy the selection (#sel copy 0 0 0)
  4. Break the log and the temporary block
  5. Step back from the selection
  6. Paste the selection pack (#sel paste 0 0 0)

The important parts are

  1. The log cannot be placed against the ground
  2. You are too far away to place the log right away
commented
ApproxPlaceable is: [minecraft:log[axis=y,variant=jungle], minecraft:air, minecraft:air, minecraft:air, minecraft:air, minecraft:air, minecraft:air, minecraft:air, minecraft:air, minecraft:air, minecraft:air, minecraft:snow_layer[layers=1], minecraft:carpet[color=white], minecraft:air, minecraft:log[axis=y,variant=jungle], minecraft:air, minecraft:air, minecraft:air, minecraft:air, minecraft:air, minecraft:air, minecraft:air, minecraft:air, minecraft:air, minecraft:air, minecraft:air, minecraft:air, minecraft:air, minecraft:air, minecraft:air, minecraft:air, minecraft:air, minecraft:air, minecraft:air, minecraft:air, minecraft:air]

This clearly shows it did notice it can place some log (axis=y), but not the one it needs (axis=x), which shouldn't matter because buildIgnoreProperties is set to axis.
For the case where rotation is irrelevant this can be fixed by changing the check, however this will still cause problems when rotation is relevant, even if there is a block to place against.

commented

Oh I see. I deleted the previous question after a quick google search but you answered quick :)

commented

I got a fix for this problem, but I'll wait with the pr until tomorrow so I have some time to check things.

commented

Big nice, I'm looking forward to it! Thanks for taking the time to look into this, and so quick too lol