Gunivers / Bookshelf

📚 Bookshelf (formerly known as Gunivers Libs) is a user-friendly modular library datapack, crafted to assist mapmakers in effortlessly implementing intricate systems within their maps.
http://bookshelf.docs.gunivers.net
Mozilla Public License 2.0
34 stars 12 forks source link

Add capability to create new message log format #183

Closed theogiraudet closed 1 month ago

theogiraudet commented 2 months ago

This PR resolves #170.

aksiome commented 2 months ago

Do we really have to set the log format on each load module? Like can we not just use the default config for all of bookshelf. Also I'm not sure im a fan of having a default namespace, that means the namespace called "default" cannot really exists and we're exposed a little bit to unintended side effects. Maybe just hardcoding the default format inside each log is simplier? Last thing, personally i think we should consider this format instead:

data modify storage bs:const log.messages set value [{namespaces:["anamespace","anotherone"],format:'["textcomponent here"]'}]
# we could use this syntax to add a new one
data modify storage bs:const log.messages[{namespaces:["mynamespace"]}].format set value '["textcomponent"]'
# or even add a namespace to a format
data modify storage bs:const log.messages[{namespaces:["mynamespace"]}].namespaces append value "otherone"

This way we can easily define multiple namespaces for a single format.

theogiraudet commented 2 months ago

Do we really have to set the log format on each load module?

It is what we discussed on Discord yeah. If we want to have a special log format for the whole Bookshelf, we need to redefine it for each module using the logger. It's also a good example of how to create a customized (complex) log format for interested people.

Also I'm not sure im a fan of having a default namespace, that means the namespace called "default"

I can move the default formats to another path to enable the "default" namespace.

Maybe just hardcoding the default format inside each log is simpler?

I think it is better to have a system like that:

data modify storage bs:const log.messages set value [{namespaces:["anamespace","anotherone"],format:'["textcomponent here"]'}]

Here you forgot to specify the level. I assume you mean:

data modify storage bs:const log.messages set value [{namespaces:["anamespace","anotherone"],format: {info: '["textcomponent here"]'}}]

If yes, so why not.

aksiome commented 2 months ago

It is what we discussed on Discord yeah. If we want to have a special log format for the whole Bookshelf, we need to redefine it for each module using the logger. It's also a good example of how to create a customized (complex) log format for interested people.

But do we really need a special one? Having an example is nice tho. Also with the array version it's a lot easier to add each namespace to the bs format.

I can move the default formats to another path to enable the "default" namespace.

Maybe if we use the array version we can have the first entry be the default one without even specifying a namespace, and/or having an other key like default:1b to target it (who knows some people may prepend to the array)

Here you forgot to specify the level. I assume you mean:

Yes exactly did not think of the level but it work the same

theogiraudet commented 1 month ago

But do we really need a special one? Having an example is nice tho. Also with the array version it's a lot easier to add each namespace to the bs format.

I think so. It makes it easy to see which logs are from Bookshelf and which are not.