Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Queue #7064

Merged
merged 14 commits into from
Jan 1, 2025
Merged

Queue #7064

merged 14 commits into from
Jan 1, 2025

Conversation

Moderocky
Copy link
Member

@Moderocky Moderocky commented Sep 9, 2024

Description

Please note: this is just one of a number of experiments, it may never see the light of day.

We were poking around with some collection ideas.
I thought about a queue, because it's annoying to make with list variables.
Removing an element leaves its index open, meaning the next addition to the list will fill the empty spot, so you'd have to re-set the list each time you change an element.

The queue is a First In, First* Out collection. That means you take items out from the start, and add them to the end.
It's a regular type rather than a variable, and you don't have direct access to its indices in terms of setting and getting as you might with a Skript list.
Queues are super fast for adding to and removing from, but less efficient for random access.

These queues don't support empty (null) values either, so adding something that doesn't exist will have no effect. A queue should never contain an empty (null) value either, which means that if you ask for a value and you get nothing, the queue must be empty.

Creating a queue

set {queue} to a new queue
set {queue} to a new queue of {things::*}
set {queue} to a new queue of "a", "b" and "c"

Adding to a queue

set {queue} to a new queue
add "hello" to {queue}
add {words::*} to {queue}
# really not rocket science, this...

It's also possible to add specifically to the start (or the end) of a queue.

set {queue} to a new queue
add "world" to {queue}
add "hello" to the start of {queue}
# values in order will be hello and world

Reading a queue

This uses ExprElement, but popping a queue removes the element you ask for.

set {thing} to the first element of {queue}
broadcast the first element of {queue}
broadcast the first element of {queue}
# each time we do this, it's popping the next value out!

set {words::*} the first 6 elements of {queue}
# removes the first 6 values

broadcast the last element of {queue}
# bye bye last value

You can also "peek" at the starting and ending values of a queue.

set {thing} to the start of {queue}
set {thing} to the end of {queue}
# this "sees" the element but doesn't remove it

Dequeueing

Queues can be converted to/from list variables easily.

set {items::*} to unqueued {queue}
set {queue} to a new queue of {items::*}
set {items::*} to dequeued {queue}

Neither of these deletes or affects the other, it just rolls/unrolls the values.

Checking whether a queue is empty

This has compatibility with the is empty condition.

if {queue} is not empty:
    # ...

Asking for an element of the queue will also return nothing if it is empty.

Indices & Setting

You cannot (currently) get the indices of an element, set an element at an index, or check whether a queue contains something.
This is not designed to be an equivalent/replacement list type, and these operations are all fairly inefficient for linked collections so I didn't want to encourage their (mis)use. You can, of course, still swap between queue -> list -> queue and do them all that way (at your own peril!)

Notes

The intentions around it were to make some odd bits and pieces easier, such as:

set {queue} to a queue of all players

while <the game is running>:
    set {player} to the first element of {queue}
    # it's player's turn now!!!
    # do their board game stuff
    # player had their turn
    add {player} to {queue}
    # now they're at the back of the queue

It'll definitely be a lot easier/more efficient than list variables for some things, but I also don't want this to be abused as a list-replacement so I'll have to consider whether it's a safe feature to add.


Target Minecraft Versions: any
Requirements: none
Related Issues: #371

@Moderocky Moderocky marked this pull request as draft September 9, 2024 21:13
@Moderocky Moderocky force-pushed the queue branch 2 times, most recently from acee7b2 to 6e0683e Compare September 9, 2024 21:17
@sovdeeth
Copy link
Member

sovdeeth commented Sep 11, 2024

I've gone back and forth on this in my head, but I think that this approach is probably the best, as opposed to new variable types. It's a bit of a departure for people (me) used to lists and how they look in skript, but considering how other languages approach data structures it makes a lot more sense for us to just have one variable type and multiple structures that can go inside them, with common syntaxes to access them. I would eventually like to see us rework the list syntax into a more generic structure (perhaps {list::index} could access maps, arrays, or queues: {map::key}, or {array::1}, or {queue::first}), but that's out of scope here.

I'll do a code review shortly, just wanted to share my high level thoughts.

Copy link
Member

@sovdeeth sovdeeth left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

looks solid, though i'd like more tests for invalid behavior like nulls, and docs for the queue classinfo

@sovdeeth sovdeeth added the feature Pull request adding a new feature. label Sep 12, 2024
@sovdeeth sovdeeth mentioned this pull request Sep 13, 2024
1 task
@Moderocky Moderocky marked this pull request as ready for review November 21, 2024 15:33
@Moderocky Moderocky requested a review from a team November 21, 2024 15:34
Copy link
Member

@APickledWalrus APickledWalrus left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Here's what I found. As for whether this should be implemented, I think it is worth consideration.

Also, does this support size of {queue}?

Copy link
Member

@APickledWalrus APickledWalrus left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

A few other things. This is shaping up nicely

@sovdeeth sovdeeth added the 2.10 Targeting a 2.10.X version release label Dec 17, 2024
public boolean init(Expression<?>[] expressions, int pattern, Kleenean delayed, ParseResult result) {
if (!this.getParser().hasExperiment(Feature.QUEUES))
return false;
if (result.hasTag("contents"))
Copy link
Member

@APickledWalrus APickledWalrus Dec 31, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

bump (this can be replaced with an expressions[0] != null check

@Moderocky Moderocky added the feature-ready A PR/issue that has been approved, tested and can be merged/closed in the next feature version. label Dec 31, 2024
Moderocky and others added 7 commits January 1, 2025 13:49
Co-authored-by: Patrick Miller <apickledwalrus@gmail.com>
Co-authored-by: Efnilite <35348263+Efnilite@users.noreply.github.com>
Co-authored-by: Patrick Miller <apickledwalrus@gmail.com>
@Moderocky Moderocky merged commit 2730adb into SkriptLang:dev/feature Jan 1, 2025
5 checks passed
@Moderocky Moderocky deleted the queue branch January 1, 2025 14:20
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
2.10 Targeting a 2.10.X version release feature Pull request adding a new feature. 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