-
Notifications
You must be signed in to change notification settings - Fork 164
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
Loot desert temples & get to outer end islands #191
Conversation
…g to `@gamer` soon
No bugs found... yet...
Now it should work ok... as far as I know...
…tape These are amazing changes imo
Removed the unused imports in `LootContainerTask`
Github is weird so I have to do this in two parts
self explanitory!
src/main/java/adris/altoclef/tasks/container/LootContainerTask.java
Outdated
Show resolved
Hide resolved
src/main/java/adris/altoclef/tasks/container/LootContainerTask.java
Outdated
Show resolved
Hide resolved
src/main/java/adris/altoclef/tasks/container/LootContainerTask.java
Outdated
Show resolved
Hide resolved
src/main/java/adris/altoclef/tasks/speedrun/BeatMinecraft2Task.java
Outdated
Show resolved
Hide resolved
src/main/java/adris/altoclef/tasks/speedrun/BeatMinecraft2Task.java
Outdated
Show resolved
Hide resolved
- Added ability to filter itemstacks in `LootContainerTask` using predicates - Removed curse of binding filter as the feature listed above can take care of that - Put duplicated code in both `RavageDesertTemplesTask` AND `LocateDesertTempleTask` in `WorldHelper`
Oh what the heck the commit formatted weird |
Updated so any screen handler works
Neater code (and the slightest bit optimized)
Aight all the suggestions are done |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I didn't review all the files as I ran out of time, but here are my main suggestions. I will re-review after the requested changes are made. Try to keep it simple and avoid unnecessary steps. It can get you stuck later on.
If you feel the requested changes don't align, please let me know or dismiss the review.
|
||
@Override | ||
protected Task onTick(AltoClef mod) { | ||
if (isFinished(mod)) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I am not sure you need this here at all. If this
is finished, it will end onTick
anyway, so something else is going on if this was the solution to a problem.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I would remove this as it may cause unexpected problems
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I mean sure
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Wait would this cause a problem with isActive
?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nah, isActive
is true when a task is running and false
when it has been interrupted/replaced by another task.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ok
if (mod.getBlockTracker().anyFound(Blocks.END_GATEWAY)) { | ||
if (!mod.getItemStorage().hasItemInventoryOnly(Items.ENDER_PEARL)) { | ||
setDebugState("Getting an ender pearl"); | ||
return new CataloguedResourceTask(new ItemTarget(Items.ENDER_PEARL, 1)); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This may already work just fine, so take this comment as a suggestion rather than a change requisition.
you may want to get the enderman hashmap from mob defence so that you can always find one. Could be wrong here.
Also, you should get more than 1 pearl. You don't know how many islands you will need to hop.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Not thinking longterm yet, just want to get to the outer end island
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Don't enderman spawn everywhere in the outer islands? If you need pearls just find one there
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Exactly
return new CataloguedResourceTask(new ItemTarget(Items.ENDER_PEARL, 1)); | ||
} | ||
BlockPos gateway = mod.getBlockTracker().getNearestTracking(Blocks.END_GATEWAY).get(); | ||
int blocksNeeded = Math.abs(mod.getPlayer().getBlockY() - gateway.getY()) + |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This was smart, although you should get at least 15 more blocks than needed. Experience has taught me that altoclef sometimes uses more blocks than it really needs. In these cases, it is good to have a backup.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I would change this
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why would it? Works perfectly fine for me. In fact, its using way less because of this sick new invention called walking on the ground so I feel like I'm already doing a backup
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I guess the problem is what if the bot has pillared up but it doesn't have enough blocks or something, it will try to mine the pillar. If half way down the pillar the bot decides "we have enough!" that's a problem since it will get stuck looping. We want the bot to finish mining the pillar and gather some materials from the ground, and that's why we usually ask for extra blocks.
If you for sure don't need that and know it won't get stuck (even in edge cases) then that's fine lol
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
No it won't, because the amount of building materials it wants updates every time the bot changes block position so if mining the pillar moves it away from the gateway, it will still try to get more
} | ||
GoalAnd goal = makeGoal(gateway); | ||
Debug.logMessage(mod.getPlayer().getBlockPos().toString()); | ||
if (!goal.isInGoal(mod.getPlayer().getBlockPos()) || !mod.getPlayer().isOnGround()) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Out of curiosity, why re-invent the wheel? You could use Altoclef's built in gotoXZ and gotoY functions for this. I would change this.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I dunno I thought it would be fun and a bit more efficient since I'm allowing baritone to approach from any side
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Also its fun to reinvent the wheel
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
agreed, it can be fun as heck
setDebugState("Getting close to gateway..."); | ||
return null; | ||
} | ||
setDebugState("Throwing the pearl inside"); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Come to think of it, why don't we throw the portal in from the ground? Why do we build to it at all? Altoclef is very good at calculating trajectories. I would suggest playing into its strengths to avoid building entirely if you can.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I was thinking this, but I wasn't sure on how to tell if we can even hit the gateway from the position we are at
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yeah there is no "parabola raycast" in yet (maybe should add one but oh well), so what you're doing is valid.
When the bot throws an ender pearl for the dragon fight it assumes there's nothing in the way and that it always will have a clear shot. Here it's different.
|
||
@Override | ||
public boolean isFinished(AltoClef mod) { | ||
return WorldHelper.getCurrentDimension() == Dimension.END && |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I would have a flag for if it successfully completed entering the portal as well. Are end islands always 400 meters away? Again, take this comment as a suggestion rather than change requisition.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
No, they're much longer but may as well be sure. I'll add a flag that's a good idea
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actually, I'm not 100% sure how to do that in the case that it fails, like if we suddenly teleport? That's probably as reliable as this solution
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This seems fine. Minor changes, I'd use WorldHelper.inRangeXZ
so it ignores your y position and a constant at the top of the file (ex. private static final int OUTER_END_ISLAND_START_RADIUS = 400
or a shorter name if you want)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actually I did a bit of research and it starts around 1000 blocks away so I'll make it 800 just to be safe
return "Going to outer end islands"; | ||
} | ||
|
||
private GoalAnd makeGoal(BlockPos gateway) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't understand the need for the offsets. IIRC, you mentioned this is to avoid getting stuck and/or hitting the bedrock. That sounds like a pearl recalculation to me. This may work, but it would be a good idea to think about why the first attempt failed. Sometimes duct tape solutions can cause further problems down the road. Since this works, take this as a suggestion.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Aight
Also removed random duct I didn't even know I had there
This method checks if the chest at that position was ever opened
…eRuinedPortalsTask`
Just in case we need it... ;)
- Use `WorldHelper.isInRange` instead of `Vec3d.distanceTo` - Make the end island radius a public constant
When using `@gamer`, we won't take anything with curse of binding, for the future ;) Also we prioritize pickaxes (I think?) when ravaging desert temples And a little neat fix to a problem when ravaging ruined portals
If we get hit by a shulker and rise high in the sky, we don't wanna die once we fall OMG AS IM TYPING THIS I REALIZED THAT WATER BUCKETS ARE THE EASY SOLUTION Well may as well... right?
Runnable as `@test iron`
Because no
No seriously, it just sticks to one bee nest, I will definitely try to optimize it sometime in the future
Do I have OCD?
Because its easier to use `WorldHelper.isBlock(mod, pos, block)` over `mod.getWorld().getBlockState(pos).getBlock() == block`
Main changes
RavageDesertTemplesTask
, runnable as@test temples
, will search deserts for temples to lootBeatMinecraft2Task
, if any desert temples are found, they will be looted for anything neededLootContainerTask
improvements, such as not looting armor with curse of bindingGetToOuterEndIslandsTask
, runnable as@test outer
, will runBeatMinecraft2Task
until it finds an end gateway, it will then get an ender pearl and blocks to pearl to the outer end islands.ConstructIronGolemTask
, runnable as@test iron
, will build an iron golem.Other changes
StorageHelper.getBrewingStandFuel()
andBrewingStandSlot