Skip to content

Conversation

@TheElenium
Copy link

Type of change

  • Bug fix
  • New feature

Description

Added a feature to the BlockESP module to allow players to filter searched block by nbt data (e.g. waterlogged, facing=north/west etc.). Mainly implemented it, because i wanted an easier way to find ominous trial vaults, but the normal esp modules had no option to distinguish between a normal and an ominous, since they are the same block (minecraft:vault).

Related issues

None

How Has This Been Tested?

image image

I created some blocks that only differ in the nbt data and filtered the in the blockesp module for those tags in the specific block configs

Checklist:

  • My code follows the style guidelines of this project.
  • I have added comments to my code in more complex areas.
  • I have tested the code in both development and production environments.

Copy link
Collaborator

@crosby-moe crosby-moe left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

  1. 'NBT Data' is the wrong nomenclature here since they only apply to block entities & entities, you should call them 'Properties' instead
  2. property filters should apply to all blocks
  3. please dont touch the formatting on lines you dont otherwise modify

Comment on lines 108 to 111
private final Map<
Block,
Map<Property<?>, Comparable<?>>
> activeFilterCache = new HashMap<>();
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this map is always empty

Comment on lines 117 to 128
sgFilters.add(new StringListSetting.Builder()
.name("NBT-Data")
.description("Filters with states (e.g. 'waterlogged=false', 'facing=north', 'ominous=true'). Only blocks matching ALL filters will be shown.")
.defaultValue(new ArrayList<>())
.onModuleActivated(stringSetting -> stringSetting.set(blockData.stateFilters))
.onChanged(filters -> {
blockData.stateFilters.clear();
blockData.stateFilters.addAll(filters);
onChanged();
})
.build()
);
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

imho using a string list for this is super unintuitive, it should be a separate setting type

public boolean tracer;
public SettingColor tracerColor;

public List<String> stateFilters = new ArrayList<>();
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

should be final

Comment on lines 329 to 354
private boolean matchesStateFilters(BlockState state, Block block, List<String> filters) {
for (String filter : filters) {
try {
// Parse "key=value" format
String[] kv = filter.split("=");
if (kv.length != 2) continue;

String propertyName = kv[0].trim();
String expectedValue = kv[1].trim();

Property<?> property = block.getStateManager().getProperty(propertyName);
if (property == null) continue;

Optional<?> parsedValue = property.parse(expectedValue);
if (parsedValue.isEmpty()) continue;

if (!state.get(property).equals(parsedValue.get())) {
return false;
}
} catch (Exception e) {
// Invalid filter format
}
}

return true;
}
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this is a hot method, parsing should be done ahead of time

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

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants