Skip to content

Multilined Conditional + Order Debug Fix#7431

Merged
APickledWalrus merged 9 commits into
SkriptLang:dev/featurefrom
Absolutionism:dev/DebugFix
Apr 1, 2025
Merged

Multilined Conditional + Order Debug Fix#7431
APickledWalrus merged 9 commits into
SkriptLang:dev/featurefrom
Absolutionism:dev/DebugFix

Conversation

@Absolutionism
Copy link
Copy Markdown
Contributor

@Absolutionism Absolutionism commented Jan 14, 2025

Description

This PR aims to fix the skipped debugging of embedded multilined conditions such as SecConditional by debugging the conditions within init which is caught in the active RetainingLogHandler and is printed in the correct spot via my fix for the order.
Also fixes the order of debug messages due to Section.parse and handler.printlog within ScriptLoader#loadItems which was placing all embedded elements above the debugged line of the parent SectionNode.

Example Code:

on load:
	if all:
		1 is greater than 2
		3 is less than 5
	then:
		broadcast "HI"

	if true is false:
		broadcast "BYE"

Before:
idea64_e9iwt4cEYj

After:
idea64_nKL1Vxc3gK

Example Code from #7236

on break:
	if player's tool is a diamond sword:
		loop blocks in radius 3 around player:
			send "hi" to player

idea64_cXm1KWRQGV


Target Minecraft Versions: any
Requirements: none
Related Issues: #7236 , #7237

@APickledWalrus APickledWalrus self-requested a review January 14, 2025 05:48
@APickledWalrus
Copy link
Copy Markdown
Member

We should look into ways for the condition debug messages to be printed from the syntax classes themselves. The ScriptLoader shouldn't have knowledge of any specific syntax implementations

@Absolutionism
Copy link
Copy Markdown
Contributor Author

Absolutionism commented Jan 14, 2025

We should look into ways for the condition debug messages to be printed from the syntax classes themselves. The ScriptLoader shouldn't have knowledge of any specific syntax implementations

That was my original plan, however, when I tried it, adding newlines for each of the condition, obviously did not retain the indentation, and when I had tried (granted I may have done it wrong) but trying getParser().getIndentation() did not align correctly (I cant really remember from my attempt)

You say that it shouldn't have knowledge of what it implements, but I'm not entirely sure what you mean. Because currently it does have knowledge that the TriggerItem is a section or statement. Which then is what allows the instanceof to be used.

@APickledWalrus
Copy link
Copy Markdown
Member

You can probably grab the indentation from the Node object itself. As for "not having knowledge", while the ScriptLoader should understand the concept of sections, statements, etc. (i.e. the syntax types), it should not have knowledge of (or reference) specific implementations of those types (e.g. conditional sections).

@Absolutionism
Copy link
Copy Markdown
Contributor Author

Absolutionism commented Jan 14, 2025

You can probably grab the indentation from the Node object itself. As for "not having knowledge", while the ScriptLoader should understand the concept of sections, statements, etc. (i.e. the syntax types), it should not have knowledge of (or reference) specific implementations of those types (e.g. conditional sections).

Alrighty, well I'll delete the MultilinedConditional interface and the instanceof + following statements within ScriptLoader
Should/Can I add a static + instance public #multilinedToString or #toMultilinedString within Conditional allowing a set method to be used to retrieve a string spanning over the appropriate amount of lines? So it can be used by addons and if/when my While All/Any?

@Absolutionism
Copy link
Copy Markdown
Contributor Author

@APickledWalrus Since the discussion on discord came to an end without a final verdict. Let me know what approach I should do.

Comment thread src/main/java/ch/njol/skript/ScriptLoader.java
Comment thread src/main/java/ch/njol/skript/sections/SecConditional.java
@sovdeeth sovdeeth added the bug An issue that needs to be fixed. Alternatively, a PR fixing an issue. label Jan 25, 2025
@Absolutionism Absolutionism requested a review from a team as a code owner March 19, 2025 22:30
@sovdeeth sovdeeth removed the request for review from a team March 21, 2025 01:49
Comment thread src/main/java/ch/njol/skript/sections/SecConditional.java
@erenkarakal erenkarakal added the feature-ready A PR/issue that has been approved, tested and can be merged/closed in the next feature version. label Mar 31, 2025
@APickledWalrus APickledWalrus merged commit 82189cf into SkriptLang:dev/feature Apr 1, 2025
erenkarakal pushed a commit to erenkarakal/Skript that referenced this pull request Nov 26, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bug An issue that needs to be fixed. Alternatively, a PR fixing an issue. feature-ready A PR/issue that has been approved, tested and can be merged/closed in the next feature version.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants