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 option to use neither node highlighting nor outlining #5758

Closed
wants to merge 4 commits into
base: master
from
Jump to file or symbol
Failed to load files and symbols.
+26 −14
Diff settings

Always

Just for now

@@ -25,7 +25,8 @@ local labels = {
},
node_highlighting = {
fgettext("Node Outlining"),
fgettext("Node Highlighting")
fgettext("Node Highlighting"),
fgettext("None")
},
filters = {
fgettext("No Filter"),
@@ -52,7 +53,7 @@ local dd_options = {
},
node_highlighting = {
table.concat(labels.node_highlighting, ","),
{"box", "halo"}
{"box", "halo", "none"}
},
filters = {
table.concat(labels.filters, ","),
Copy path View file
@@ -344,7 +344,7 @@ enable_clouds (Clouds) bool true
enable_3d_clouds (3D clouds) bool true
# Method used to highlight selected object.
node_highlighting (Node highlighting) enum box box,halo
node_highlighting (Node highlighting) enum box box,halo,none
# Adds particles when digging a node.
enable_particles (Digging particles) bool true
Copy path View file
@@ -382,7 +382,7 @@
# enable_3d_clouds = true
# Method used to highlight selected object.
# type: enum values: box, halo
# type: enum values: box, halo, none
# node_highlighting = box
# Adds particles when digging a node.
Copy path View file
@@ -87,24 +87,31 @@ Hud::Hud(video::IVideoDriver *driver, scene::ISceneManager* smgr,
m_halo_boxes.clear();
m_selection_pos = v3f(0.0, 0.0, 0.0);
std::string mode = g_settings->get("node_highlighting");
std::string mode_setting = g_settings->get("node_highlighting");

This comment has been minimized.

@nerzhul

nerzhul May 13, 2017

Member

const std::string &

This comment has been minimized.

@Zeno-

Zeno- May 13, 2017

Contributor

What for?

This comment has been minimized.

@SmallJoker

SmallJoker May 13, 2017

Member

Speed, that we don't neccessarly need here.

This comment has been minimized.

@Ezhh

Ezhh May 13, 2017

Member

So not necessarily needed, but is it wanted?

This comment has been minimized.

@Zeno-

Zeno- May 13, 2017

Contributor

Look, IMO it's not necessary here because it's in the constructor and will not affect performance at all. But if it makes people happy just change it ;)

This comment has been minimized.

@paramat

paramat May 13, 2017

Member

I thought about this too, since it's in the constructor a const does not make a difference and is not needed.

This comment has been minimized.

@Ezhh

Ezhh May 13, 2017

Member

This seems to be three not neededs, so unless I get a clear request to change, I'm leaving it as is for now.

This comment has been minimized.

@nerzhul

nerzhul May 14, 2017

Member

it's not just const but const reference @paramat , it permit to prevent a useless copy. it's not just constness ,but memory reduction & copy

This comment has been minimized.

@paramat

paramat May 15, 2017

Member

Ah missed the '&', sorry.

This comment has been minimized.

@Nocticorvus

Nocticorvus May 15, 2017

Suuuuuuper Needed for ascetics and video production alike... Loads more Minetest related content on YouTube may surface due to the option being present. Some art is designed in great detail and the Black lines or alt. color upon closer inspection make that ART look terrible. I think everyone will agree once you get past the rainbow wool hut stage it's nothing but art from there and as time passes it gets more and more intricate. People want to show off their art and some will go so far as to prominently display it in the largest way possible via pubic accessible posting or videos. But only if the "close inspection" is up to par with the entirety of the workings itself. This may seem like an Odd observation but I am an odd person in all reality and feel that this is a great addition to the Minetest world. Another Great suggestion by the Great Ezhh :

if (mode_setting == "halo") {
m_mode = HIGHLIGHT_HALO;
} else if (mode_setting == "none") {
m_mode = HIGHLIGHT_NONE;
} else {
m_mode = HIGHLIGHT_BOX;
}
m_selection_material.Lighting = false;
if (g_settings->getBool("enable_shaders")) {
IShaderSource *shdrsrc = client->getShaderSource();
u16 shader_id = shdrsrc->getShader(
mode == "halo" ? "selection_shader" : "default_shader", 1, 1);
m_mode == HIGHLIGHT_HALO ? "selection_shader" : "default_shader", 1, 1);
m_selection_material.MaterialType = shdrsrc->getShaderInfo(shader_id).material;
} else {
m_selection_material.MaterialType = video::EMT_TRANSPARENT_ALPHA_CHANNEL;
}
if (mode == "box") {
m_use_selection_mesh = false;
if (m_mode == HIGHLIGHT_BOX) {
m_selection_material.Thickness =
rangelim(g_settings->getS16("selectionbox_width"), 1, 5);
} else if (mode == "halo") {
m_use_selection_mesh = true;
} else if (m_mode == HIGHLIGHT_HALO) {
m_selection_material.setTexture(0, tsrc->getTextureForMesh("halo.png"));
m_selection_material.setFlag(video::EMF_BACK_FACE_CULLING, true);
} else {
@@ -518,7 +525,7 @@ void Hud::setSelectionPos(const v3f &pos, const v3s16 &camera_offset)
void Hud::drawSelectionMesh()
{
if (!m_use_selection_mesh) {
if (m_mode == HIGHLIGHT_BOX) {
// Draw 3D selection boxes
video::SMaterial oldmaterial = driver->getMaterial2D();
driver->setMaterial(m_selection_material);
@@ -538,7 +545,7 @@ void Hud::drawSelectionMesh()
driver->draw3DBox(box, video::SColor(255, r, g, b));
}
driver->setMaterial(oldmaterial);
} else if (m_selection_mesh) {
} else if (m_mode == HIGHLIGHT_HALO && m_selection_mesh) {
// Draw selection mesh
video::SMaterial oldmaterial = driver->getMaterial2D();
driver->setMaterial(m_selection_material);
@@ -564,7 +571,7 @@ void Hud::drawSelectionMesh()
void Hud::updateSelectionMesh(const v3s16 &camera_offset)
{
m_camera_offset = camera_offset;
if (!m_use_selection_mesh)
if (m_mode != HIGHLIGHT_HALO)
return;
if (m_selection_mesh) {
Copy path View file
@@ -175,7 +175,11 @@ class Hud {
v3f m_selected_face_normal;
video::SMaterial m_selection_material;
bool m_use_selection_mesh;
enum {

This comment has been minimized.

@nerzhul

nerzhul May 13, 2017

Member

it's not C. enum Mode { };

This comment has been minimized.

@Zeno-

Zeno- May 13, 2017

Contributor

No, this is incorrect. It's meant to be an anonymous enum

This comment has been minimized.

@Ezhh

Ezhh May 13, 2017

Member

Happy to change if needed - but please let me know what the agreement is.

This comment has been minimized.

@Zeno-

Zeno- May 13, 2017

Contributor

enum Mode {}; creates a non-anonymous enum and that, I believe, is not needed nor required here.

This comment has been minimized.

@Ezhh

Ezhh May 13, 2017

Member

I'm going to take that to mean this change isn't needed for now. Let me know if it is.

HIGHLIGHT_BOX,
HIGHLIGHT_HALO,
HIGHLIGHT_NONE } m_mode;
};
enum ItemRotationKind {
ProTip! Use n and p to navigate between commits in a pull request.