[CSM] Add basic HUD manipulation. #6067

red-001 · 2017-06-28T01:37:41Z

Adds CSM bindings for the core HUD manipulation commands and some sample/test lua code. See docs for a list of added functions.

paramat · 2017-06-28T04:16:08Z

~~Presumably HUD flags from the server still control whether the minimap is usable?~~

nerzhul · 2017-06-28T07:54:59Z

@red-001 very nice implementation, it will permit to have custom Hud client side to show various local things without involving server

orwell96 · 2017-06-28T11:34:03Z

src/network/clientpackethandler.cpp

-
+ u32 client_id = getEnv().getLocalPlayer()->getFreeHudID();
+ m_hud_server_to_client[server_id] = client_id;
+ errorstream << "Add " << "client id:" << client_id << " server id:" << server_id << std::endl;


Why errorstream?

oh sorry that's test code that I forgot about.

You might have left it in, but to the infostream or debugstream

red-001 · 2017-06-28T11:55:21Z

@paramat yes

nerzhul · 2017-06-30T13:40:23Z

src/network/clientpackethandler.cpp

- event.type = CE_HUDRM;
- event.hudrm.id = id;
- m_client_event_queue.push(event);
+ std::unordered_map<u32, u32>::iterator i = m_hud_server_to_client.find(server_id);


const_iterator

nerzhul · 2017-06-30T13:40:29Z

src/network/clientpackethandler.cpp

- event.hudchange.data = intdata;
- event.hudchange.v2s32data = new v2s32(v2s32data);
- m_client_event_queue.push(event);
+ std::unordered_map<u32, u32>::iterator i = m_hud_server_to_client.find(server_id);


nerzhul · 2017-06-30T13:41:31Z

src/hud.cpp

+ {HUD_FLAG_MINIMAP_VISIBLE, "minimap"},
+ {0, NULL},
+};
+#ifndef SERVER


i don't like this ifndef. Please add the moved code to a common header between client and server. We really should remove this dependency between client/server parts which are near renderer

Dumbeldor · 2017-06-30T23:23:48Z

src/script/lua_api/l_localplayer.cpp

+{
+ LocalPlayer *player = getobject(L, 1);
+ u32 id = luaL_checkinteger(L, 2);
+ HudElement* element = player->removeHud(id);


HudElement *element

Dumbeldor · 2017-06-30T23:24:09Z

src/script/lua_api/l_localplayer.cpp

+ LocalPlayer *player = getobject(L, 1);
+ u32 id = luaL_checkinteger(L, 2);
+ HudElement* element = player->removeHud(id);
+ if (element == NULL)


if (!element)

Dumbeldor · 2017-06-30T23:24:43Z

src/script/lua_api/l_localplayer.cpp

+ if (!e)
+ return 0;
+
+ void *unused;


void *unused = nullptr; ?

paramat · 2017-08-25T22:53:25Z

Please add a description with a summary of what this does.

nerzhul · 2017-08-26T07:37:28Z

i think i will adopt it.
@paramat this PR add CSM Hud , it permits, like in many popular MMORPG to create custom Hud client side with various available informations, for example with that you can create a Hud which contains the coordinates and place it under minimap, instead of having debug infos opened every time

paramat · 2017-08-27T13:06:13Z

Yeah once i looked through the code i saw it is basic HUD element support The lack of description means people misunderstand, which caused my first comment above.

The PR is not so simple that no description is needed, the basic content of a PR needs to be clear without looking at the code. RBA used to leave out the description on non-trivial PRs and it was really irritating. Even worse when one is asked for and it does not appear (deserves a -1 in those cases).

It's very easy for a PR author to add a clear summary and it really helps reviewers and managing of contributions. It should be a rule for contributions.

red-001 · 2017-08-31T06:50:25Z

@paramat @nerzhul rebased

nerzhul · 2017-08-31T10:31:43Z

src/hud.h

- const core::rect<s32> *clip,
- Client *client,
- ItemRotationKind rotation_kind);
+extern const struct EnumString es_HudElementType[];


redundant struct keywoard, we are in C++

nerzhul · 2017-08-31T10:34:40Z

can you pass clang-format on headers and add client/hud.cpp to the clang-format-whitelist file please ?

nerzhul · 2017-08-31T17:58:50Z

util/travis/clang-format-whitelist.txt

@@ -97,8 +97,10 @@ src/guiTable.h
 src/guiVolumeChange.cpp
 src/guiVolumeChange.h
 src/httpfetch.cpp
-src/hud.cpp
+src/client/hud.cpp


please respect alphabetical order on this list. Why adding src/client/hud.h ? Linter is correct on it

paramat · 2017-08-31T18:45:06Z

Thanks.

nerzhul · 2017-09-05T09:22:10Z

@SmallJoker can you review it, it should be merged ASAP it's one of the key feature of CSM, especially when mod channels will be merged

SmallJoker · 2017-09-06T17:12:55Z

doc/client_lua_api.md

+### HUD Definition (`hud_add`, `hud_get`)
+```lua
+ {
+ hud_elem_type = "image", -- see HUD element types


Defaults to HUD_ELEM_TEXT -> "text"
EDIT: Wrong defaults in position, scale, number (needs more docs too), item, size and world_pos (which is missing here).

I copied the example from the server lua api doc. I think it was meant to just be an example not a list of default values.

SmallJoker · 2017-09-06T17:19:20Z

doc/client_lua_api.md

@@ -976,6 +976,16 @@ Methods:
 * returns last look vertical angle
 * `get_key_pressed()`:
 * returns last key typed by the player
+* `hud_add(definition)`
+ * add a HUD element described by HUD def, returns ID number on success
+ * See [`HUD definition`](#hud-definition-hud_add-hud_get)


returns the HUD ID or nil, if it failed to create the HUD

SmallJoker · 2017-09-06T17:20:03Z

doc/client_lua_api.md

+ * add a HUD element described by HUD def, returns ID number on success
+ * See [`HUD definition`](#hud-definition-hud_add-hud_get)
+* `hud_get(id)`
+ * returns the [`definition`](#hud-definition-hud_add-hud_get) of the HUD with that ID number


or nil, if non-existent.

SmallJoker · 2017-09-06T17:24:33Z

doc/client_lua_api.md

+ * remove the HUD element of the specified id, returns `true` on success
+* `hud_change(id, stat, value)`
+ * change a value of a previously added HUD element
+ * element `stat` values: `position`, `name`, `scale`, `text`, `number`, `item`, `dir`


or nil, if the HUD ID is non-existent.

SmallJoker · 2017-09-06T17:25:43Z

src/script/lua_api/l_localplayer.cpp

+ if (!element)
+ lua_pushboolean(L, false);
+ else
+ lua_pushboolean(L, true);


The HUD must be deleted. i.e. delete element;

SmallJoker

See comments.

paramat · 2017-12-26T21:22:08Z

@nerzhul @SmallJoker is rebased.

SmallJoker · 2017-12-29T09:46:32Z

src/CMakeLists.txt

@@ -448,6 +448,7 @@ set(common_SRCS
 version.cpp
 voxel.cpp
 voxelalgorithms.cpp
+ hud.cpp


Please keep it sorted alphabetically.

SmallJoker · 2017-12-29T10:00:01Z

doc/client_lua_api.txt

+ * add a HUD element described by HUD def, returns ID number on success and `nil` on failure.
+ * See [`HUD definition`](#hud-definition-hud_add-hud_get)
+* `hud_get(id)`
+ * returns the [`definition`](#hud-definition-hud_add-hud_get) of the HUD with that ID number or `nil`, if non-existent.


This link may not work in a few markdown viewers due the parentheses and code expressions in the title

SmallJoker · 2017-12-29T10:06:27Z

src/network/clientpackethandler.cpp

+ ClientEvent *event = new ClientEvent();
+ event->type = CE_HUDRM;
+ event->hudrm.id = client_id;
+ m_client_event_queue.push(event);


Also remove this ID from m_hud_server_to_client when Game::handleClientEvent_HudRemove is called later.

SmallJoker · 2017-12-29T10:12:26Z

src/script/lua_api/l_localplayer.cpp

+{
+ LocalPlayer *player = getobject(L, 1);
+
+ u32 id = lua_tonumber(L, -1);


Also an issue for the SSM l_hud_get: This function returns 0 if it's not a valid number. Returning nil seems reasonable in that case.

shouldn't be an issue, in most cases returning nothing is almost the same as returning nil.
Might be easier to just change the documentation for SSM to say "return nothing if invalid"

My point is that 0 is also a valid HUD ID - this would get the wrong ID for non-numeric parameters.
EDIT: -> luaL_checkinteger

oh sorry I didn't see what you meant there, I thought you meant l_hud_get by this function and the return 0 in it which resulted in it returning nothing, when the documentation said it returns nil in case of invalid input.

red-001 · 2018-01-19T17:20:50Z

formatted the whitelist with sort just so it's at least ordered in a consistent way.

red-001 · 2018-01-20T07:26:12Z

note the last commit is only needed because previous changes to CSM seem to have broken on_connect callbacks

Workaround for on_connect not working right now.

nerzhul

okay for me

SmallJoker

👍 as soon the last comment is handled.

* [CSM] Add basic HUD manipulation. Workaround for on_connect not working right now.

nerzhul added the @ Client Script API label Jun 28, 2017

orwell96 reviewed Jun 28, 2017

View reviewed changes

nerzhul added the One approval ✅ ◻️ label Jun 30, 2017

nerzhul requested changes Jun 30, 2017

View reviewed changes

nerzhul removed the One approval ✅ ◻️ label Jun 30, 2017

Dumbeldor approved these changes Jun 30, 2017

View reviewed changes

red-001 force-pushed the csm_hud branch 2 times, most recently from 46d8bd8 to a3c8793 Compare July 5, 2017 22:12

nerzhul added this to In Progress in Server-sent Client Side Modding Jul 16, 2017

red-001 force-pushed the csm_hud branch from a3c8793 to 7f4ce33 Compare August 30, 2017 23:08

nerzhul reviewed Aug 31, 2017

View reviewed changes

red-001 force-pushed the csm_hud branch from c54dd05 to 705c8a8 Compare August 31, 2017 14:20

nerzhul previously requested changes Aug 31, 2017

View reviewed changes

SmallJoker reviewed Sep 6, 2017

View reviewed changes

SmallJoker previously requested changes Sep 6, 2017

View reviewed changes

red-001 force-pushed the csm_hud branch from 705c8a8 to 62c4e74 Compare October 4, 2017 16:33

red-001 force-pushed the csm_hud branch from db02357 to 9827c56 Compare December 26, 2017 17:32

paramat removed the Rebase needed The PR needs to be rebased by its author label Dec 26, 2017

minetest deleted a comment from ThomasMonroe314 Dec 26, 2017

SmallJoker reviewed Dec 29, 2017

View reviewed changes

red-001 added 2 commits January 19, 2018 16:48

[CSM] Add basic HUD manipulation.

dcf4114

add common hud files

0d9bb3a

red-001 force-pushed the csm_hud branch from 9827c56 to 026dd92 Compare January 19, 2018 17:20

red-001 added 2 commits January 20, 2018 11:17

Fix issues pointed out in code review

4ddae10

Make sure everything is initialized before displaying test hud

55bd703

Workaround for on_connect not working right now.

red-001 force-pushed the csm_hud branch from 026dd92 to 55bd703 Compare January 20, 2018 11:17

nerzhul added the One approval ✅ ◻️ label Jan 20, 2018

nerzhul approved these changes Jan 20, 2018

View reviewed changes

Fix android build

196a071

SmallJoker reviewed Jan 20, 2018

View reviewed changes

Check arguments to hud_get and throw an error if they are incorrect.

116106d

SmallJoker approved these changes Jan 20, 2018

View reviewed changes

SmallJoker added >= Two approvals ✅ ✅ and removed One approval ✅ ◻️ labels Jan 20, 2018

nerzhul merged commit 9649e47 into minetest:master Jan 20, 2018

t0ny2 pushed a commit to t0ny2/minetest that referenced this pull request Jan 23, 2018

[CSM] Add basic HUD manipulation. (minetest#6067)

4e6c718

* [CSM] Add basic HUD manipulation. Workaround for on_connect not working right now.

paramat moved this from In Progress to Done in Server-sent Client Side Modding Jul 19, 2018

osjc pushed a commit to osjc/minetest that referenced this pull request Jan 11, 2019

[CSM] Add basic HUD manipulation. (minetest#6067)

8ddc403

* [CSM] Add basic HUD manipulation. Workaround for on_connect not working right now.

osjc pushed a commit to osjc/minetest that referenced this pull request Jan 23, 2019

[CSM] Add basic HUD manipulation. (minetest#6067)

a3214c3

* [CSM] Add basic HUD manipulation. Workaround for on_connect not working right now.

neoh4x0r mentioned this pull request Jun 13, 2021

make item count background work #11316

Closed

[CSM] Add basic HUD manipulation. #6067

[CSM] Add basic HUD manipulation. #6067

Conversation

red-001 commented Jun 28, 2017 • edited Loading

paramat commented Jun 28, 2017 • edited Loading

nerzhul commented Jun 28, 2017

Choose a reason for hiding this comment

Choose a reason for hiding this comment

orwell96 Jul 1, 2017 • edited Loading

Choose a reason for hiding this comment

red-001 commented Jun 28, 2017

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Dumbeldor Jun 30, 2017 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

paramat commented Aug 25, 2017

nerzhul commented Aug 26, 2017

paramat commented Aug 27, 2017 • edited Loading

red-001 commented Aug 31, 2017

Choose a reason for hiding this comment

nerzhul commented Aug 31, 2017

Choose a reason for hiding this comment

paramat commented Aug 31, 2017

nerzhul commented Sep 5, 2017

SmallJoker Sep 6, 2017 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

SmallJoker Sep 6, 2017 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

SmallJoker Sep 6, 2017 • edited Loading

Choose a reason for hiding this comment

SmallJoker left a comment

Choose a reason for hiding this comment

paramat commented Dec 26, 2017 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

SmallJoker Dec 29, 2017 • edited Loading

Choose a reason for hiding this comment

red-001 Jan 19, 2018 • edited Loading

Choose a reason for hiding this comment

SmallJoker Jan 20, 2018 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

red-001 commented Jan 19, 2018

red-001 commented Jan 20, 2018

nerzhul left a comment

Choose a reason for hiding this comment

SmallJoker left a comment

Choose a reason for hiding this comment

red-001 commented Jun 28, 2017 •

edited

Loading

paramat commented Jun 28, 2017 •

edited

Loading

orwell96 Jul 1, 2017 •

edited

Loading

Dumbeldor Jun 30, 2017 •

edited

Loading

paramat commented Aug 27, 2017 •

edited

Loading

SmallJoker Sep 6, 2017 •

edited

Loading

SmallJoker Sep 6, 2017 •

edited

Loading

SmallJoker Sep 6, 2017 •

edited

Loading

paramat commented Dec 26, 2017 •

edited

Loading

SmallJoker Dec 29, 2017 •

edited

Loading

red-001 Jan 19, 2018 •

edited

Loading

SmallJoker Jan 20, 2018 •

edited

Loading