Skip to content
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 a detailed_description to items #8752

Closed
wants to merge 1 commit into from

Conversation

Desour
Copy link
Member

@Desour Desour commented Aug 4, 2019

  • I'm always frustrated when I want to describe an item but don't want this description to be shown always. This PR adds the field detailed_description to the item definition and also the corresponding meta field. detailed_description is used in favour of description when the same key is pressed as the one needed for listrings, shift (not sneak).
  • I think there was an issue for this. I'll have to search. Edit: I haven't found anything.
  • There are probably many usecases for this, for example special properties for tools or a detailed description on how to use the screwdriver.

Screenshot

screenshot_20190804_194358
(That mouse is not real.)

To do

This PR is a Ready for Review.
Please read my code comments, too.

How to test

Try the minimal mese pick and use the following chatcommand to test the meta field:

minetest.register_chatcommand("dd", {
	params = "<detailed_description>",
	description = "set the detailed_description",
	privs = {},
	func = function(name, param)
		local player = minetest.get_player_by_name(name)
		local item = player:get_wielded_item()
		local meta = item:get_meta()
		--~ meta:set_string("detailed_description", minetest.colorize("#abc", param))
		meta:set_string("detailed_description", param)
		player:set_wielded_item(item)
		return true, "detailed_description set to \"" .. param .. "\""
	end,
})

src/itemdef.cpp Outdated Show resolved Hide resolved
if (!i.detailed_description.empty()) {
lua_pushstring(L, i.detailed_description.c_str());
lua_setfield(L, -2, "detailed_description");
}
Copy link
Member Author

Choose a reason for hiding this comment

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

Should do this even if the detailed_description is empty?

Copy link
Contributor

Choose a reason for hiding this comment

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

This field should only be set when it isn't empty (like all other text fields, AFAIK). If this field isn't explicitly set, this would be nil, which is the correct behaviour.

@Desour Desour force-pushed the detailed_descr branch 2 times, most recently from e6e2940 to a4da5d9 Compare August 4, 2019 18:19
@paramat paramat added @ Client / Audiovisuals Feature ✨ PRs that add or enhance a feature labels Aug 4, 2019
@Desour
Copy link
Member Author

Desour commented Sep 14, 2019

Rebased.

@rubenwardy
Copy link
Member

I'm tied on this PR.

Alternatives

I'm wondering whether it's better to do this the opposite way, and have a short_description which is automatically set from the default description, as a lot of mods which set descriptions to be more than one line do so with meta.

It's worth noting that mods which show the description probably already only show the first line, if coded well This is a solution that mostly works, but isn't flexible.

Problems

It's possible that mods only want to show the shorter description on item image buttons etc, which is not allowed. There feature doesn't exist already though, so is irrelevant for this PR

@Desour
Copy link
Member Author

Desour commented Sep 14, 2019

short_description might be a good idea, old clients should always show the long descriptions and old meta-set descriptions are detailed. 🤔

@rubenwardy
Copy link
Member

I've decided that having short_description would be a much better idea. I suggest automatically creating this short_description by taking the long description and cutting all but the first line.

There is a problem - what to do if description is changed using meta but short_description isn't. I suggest ignoring this case

This has the added benefit of not needing to change the description code behind tooltips

@Desour
Copy link
Member Author

Desour commented Sep 23, 2019

I'll close this for now.

@Desour Desour closed this Sep 23, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
@ Client / Audiovisuals Feature ✨ PRs that add or enhance a feature
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants