Improve script logging #115
Conversation
|
|
||
| private static ParseableTag fromFormatScriptOrParse(String rawFormat, ScriptContainer owner) { | ||
| FormatScriptContainer formatScript = ScriptRegistry.getScriptContainerAs(rawFormat, FormatScriptContainer.class); | ||
| return formatScript != null && formatScript.getFormatTag() != null ? formatScript.getFormatTag() : TagManager.parseTextToTag(rawFormat, DenizenCore.implementation.getTagContext(owner)); |
There was a problem hiding this comment.
This is such a cursed way of differentiating two types of input.
If I input eg my_project_errar then any errors are gonna log my_project_errar because I made a typo, and I won't notice that typo til I happen to have an error in the script.
Ideally there'd be some more explicit, like, error_script: vs error: or something so if error_script is set but invalid it immediately logs an error during reload
There was a problem hiding this comment.
Yeah that was mostly an early throw-together format, changed it into the vision™ (you can now register format types, which means we could maybe add support for this in narrate, announce, etc. later on) with split keys for raw/script formats - lmk if that looks good.
| // @name Script Logging Format | ||
| // @group Script Container System | ||
| // @description | ||
| // Script logging contexts provide the format certain commands will use for their texts. |
There was a problem hiding this comment.
This meta should probably have a Most notably, this includes <@link command debug>
There was a problem hiding this comment.
Added, and clarified the format a bit in general.
src/main/java/com/denizenscript/denizencore/scripts/commands/core/DebugCommand.java
Show resolved
Hide resolved
| @@ -33,19 +37,17 @@ public DebugCommand() { | |||
| // Use to quickly output debug information to console. | |||
| // | |||
| // Valid types include: | |||
| // DEBUG: standard hideable debug. | |||
| // DEBUG: standard hideable debug, supports <@link language Script Logging Formats> (in which case it isn't hideable). | |||
There was a problem hiding this comment.
If it's not standard hideable debug, it should be a separate type. ScriptDebug or something, idk.
There was a problem hiding this comment.
Add a default OUTPUT enum value for this (lmk if the name is good, not sure honestly), but documenting it as an option felt weird, so I ended up documenting it as basically an optional arg that just technically defaults to OUTPUT in the background, but can change it if preferred.
| @@ -59,6 +60,8 @@ public class ScriptContainer implements Debuggable { | |||
| // | |||
| // --> | |||
|
|
|||
| ScriptLoggingContext loggingContext; | |||
| @@ -15,7 +27,7 @@ public void addDefinition(String definition, ObjectTag value) { | |||
|
|
|||
| @Override | |||
| public void addDefinition(String definition, String value) { | |||
| addDefinition(definition, new ElementTag(value)); | |||
| addDefinition(definition, new ElementTag(value, true)); | |||
There was a problem hiding this comment.
Noope. This is not in fact plaintext, this is for raw unparsed data.
| private final MapTag definitions; | ||
|
|
||
| public SimpleDefinitionProvider(MapTag map) { | ||
| this.definitions = map; |
There was a problem hiding this comment.
Thaaat's dangerous. This constructor should make a copy of the map, not track the original instance directly (ie the caller shouldn't have to do its own copy as you do below)
src/main/java/com/denizenscript/denizencore/scripts/containers/ScriptContainer.java
Show resolved
Hide resolved
src/main/java/com/denizenscript/denizencore/scripts/containers/core/FormatScriptContainer.java
Show resolved
Hide resolved
src/main/java/com/denizenscript/denizencore/scripts/ScriptLoggingContext.java
Outdated
Show resolved
Hide resolved
| import java.util.Map; | ||
| import java.util.Set; | ||
|
|
||
| public record ScriptLoggingContext(Map<Key, ParseableTag> formats, ParseableTag singleFormat) { |
There was a problem hiding this comment.
No specific reason, records are just an easy "Hey java, give me an object with these fields and a ready constructor and equals and everything" - afaics making this a class would just be more lines of code, no? as it never needs to be modified anyway
No description provided.