implemented logger in d2gamescreen #925
Conversation
|
I've added logger to d2game/d2player |
|
Ok, I think, this is complete |
| gw.logger = d2util.NewLogger() | ||
| gw.logger.SetLevel(l) | ||
| gw.logger.SetPrefix(logPrefix) | ||
|
|
There was a problem hiding this comment.
I think that this is problematic. Do individual widgets need logger instances?
should we not instead return these errors to the HUD and let the HUD report errors?
There was a problem hiding this comment.
yes, we can return errors to HUD, but ONLY ERRORS. What about logger.Info like in help_overlay:go:202?
| type skillIcon struct { | ||
| *d2ui.BaseWidget | ||
| lvlLabel *d2ui.Label | ||
| sprite *d2ui.Sprite | ||
| skill *d2hero.HeroSkill | ||
|
|
||
| logger *d2util.Logger | ||
| } |
There was a problem hiding this comment.
another widget that has it's own logger. maybe this is not such an issue if you assign to it an existing logger (like from the HUD or some menu)
There was a problem hiding this comment.
maybe, so could you give some example. what should I type instead of logger *d2util.Logger?
There was a problem hiding this comment.
I don't think that widgets should create loggers, they should likely use a parent logger instance (like the UI Manager logger instance).
@essial thoughts?
gravestench
dismissed
their stale review
Questionable logger instances in widgets
|
After a lot of discussion with @gucio321, it's not so bad if widgets have loggers. An alternate approach of using the logger of whatever was using the widget was considered, but was more trouble than it was worth. If there are no objections I would like to merge. |
* logger for d2audio & d2map * logger for d2ui e.t.c * d2inventory now passes on error messages * no more importing log in d2core * implemented #925 * added logger to part of d2networking & fixed "need to be changed" comments * fixed lints * fixed errors Co-authored-by: M. Sz <mszeptuch@protonmail.com>
Hi there,
no more import "log" in module d2gamescreen