Skip to content

Added get_name_from_content_id#45

Merged
S-S-X merged 2 commits intomasterfrom
content-ids
Oct 27, 2021
Merged

Added get_name_from_content_id#45
S-S-X merged 2 commits intomasterfrom
content-ids

Conversation

@S-S-X
Copy link
Owner

@S-S-X S-S-X commented Oct 23, 2021

What added

Function get_content_id was added here: #13

To be decided

As content ids get generated when you ask for one there should not be id available for name before using minetest.get_content_id(name).
This makes minetest.get_name_from_content_id(cid) generate missing ids when called with unknown content id.

Should minetest.get_name_from_content_id(cid) fail immediately if called with unknown id?
Also is content id 0 based or 1 based, probably 0 based (also used in original commit) is correct?

@S-S-X S-S-X added the enhancement New feature or request label Oct 23, 2021
@S-S-X
Copy link
Owner Author

S-S-X commented Oct 23, 2021

Ping @BuckarooBanzay as you've made original changes. Any ideas for questions stated?

Also fail / not fail with unknown things: mineunit is testing framework so excessive error/failure avoidance is of course bad thing, I think question is more about which way follows engine implementation better.

@BuckarooBanzay
Copy link
Contributor

Should minetest.get_name_from_content_id(cid) fail immediately if called with unknown id?

Fail immediately would match with the current engine behavior

Also is content id 0 based or 1 based, probably 0 based (also used in original commit) is correct?

Not sure about that one, there are also some magic numbers (126 for air for example) but i'm not sure if/how they are translated into the game 🤷
IMO: shouldn't matter, no sane person would compare the content-id's with hardcoded values (i hope)

@S-S-X
Copy link
Owner Author

S-S-X commented Oct 27, 2021

Cleaned up, minetest.get_name_from_content_id(cid) will crash if called before minetest.get_content_id(name).
This is probably fine and content ids can be generated on the fly while adding VoxelManip functionality and implementing VoxelArea #34, #35 and voxelmanip.lua

I think this is a lot more useful after mentioned things are done, currently seems to work like before without affecting behavior in any way.

@S-S-X S-S-X merged commit e32b547 into master Oct 27, 2021
@S-S-X S-S-X deleted the content-ids branch October 27, 2021 20:40
@S-S-X
Copy link
Owner Author

S-S-X commented Oct 28, 2021

Bit deeper these things might actually matter, see comment #34 (comment)

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

Labels

enhancement New feature or request

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants