pyrollo / display_modpack

Display modpack for Minetest, provides mods with dynamic display and font display : clocks, signs, and more.
GNU Lesser General Public License v3.0
25 stars 27 forks source link

Seperate signs API from signs definitions #12

Closed Thomas--S closed 6 years ago

pyrollo commented 6 years ago

That's a good idea to separate the sign api from the mod. At first, this mod was just a use case demo for display lib.

I'm thinking about names. It would be better to have homogeneous names for mods, So as we have display_lib and font_lib, signs_api shoud be signs_lib but it would conflict with minetest-mods/signs_lib.

I see tree possibilities :

And for the remaining, signs mod, I'll rename it signs_basic or something like that (this implies use of aliases for compatibility reasons).

I'm still thinking about that. What is your opion ?

Thomas--S commented 6 years ago

After thinking about this, I actually prefer display_api / font_api / signs_api.

In my opinion, the first option would cause too much confusion with the other signs mod.

The third option would solve the name problem, but I prefer to separate font_lib from signs_api. Both APIs are designed for different usages. font_lib is a more general approach, while signs_api is very specialized.

Please tell me your choice, when you have decided what option you prefer, then I'll update this PR accordingly.

Thanks!

pyrollo commented 6 years ago

Let's go for display_api / font_api / signs_api. I'll change the signs mod name the.next time I'll do significant things on it. Thank you :)

Thomas--S commented 6 years ago

Thanks for your comment! I'll update the PR accordingly tomorrow.

Thomas--S commented 6 years ago

The PR is updated now.

Please test this before merging. This is a rather large change, and I cannot guarantee that I didn't overlook a mistake. However, I tested this briefly, and it seems to work.

Thanks!

pyrollo commented 6 years ago

PR tested and merged :) Thanks for that work !

Thomas--S commented 6 years ago

Thanks for merging!