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

Play from here #2357

Open
wants to merge 14 commits into
base: main
Choose a base branch
from

Conversation

firedfromlife
Copy link
Contributor

Addressing ancient issue #1181

  • Not ready for prime time I think, but looking for feedback.
  • It works, so that is already much better than currently.
  • It works using the same logic as Jump, so all events between the first and the selected are skipped.
    • So say you have a character enter the scene on line 3 and you jump to line 5, the character will not have entered the scene.

Questions:

  • It only works in the visual editor, should I try to make it work in the text editor too?
  • I think it would be a tremendous amount of work to make this not work like jump with skipping, but would it be worth it?
    • I feel like you would need to basically load the timeline, copy any relevant events that occur prior to the jump point, somehow process those without having them actually happen for time reasons, then jump. So basically when copying all the events, set their pauses to zero and their fades to zero, etc.

firedfromlife and others added 14 commits June 19, 2024 02:31
- Character Events now suggest characters already in the timeline first.
- Text Events now suggest characters already in the timeline first.
- Text Events now place the most character from the most recent (relative to the text event) character event in the timeline first.
- character was shadowing from the outer scope, updated to event_character
	- More descriptive and less likely to shadow
- make character.get_character_name() return Unique Identifier if possible
- add check to avoid error when a character event doesn't have a character set
- Now acknowleges character and text events.
- Live updates!
- Ignores null values in characters and text events.
- Only searches up the timeline relevant to the current event.
Whether the sidebar is collapsed is now saved to editor state.

Simplifies ResourceListItem class into a contained class (avoids the public class name). Also uses RefCounted to avoid having to free them manually.

Some code cleanup.
@Jowan-Spooner
Copy link
Collaborator

@firedfromlife very cool!
I think it would be cool to have this with processing the previous events, but I can see a use for just the simple jump-like functionality as well, if that turns out to complicated.

Having it for the text editor would be nice, but is not a requirement.

I think it should be doable to use the auto_skip functionality which already allows setting a max time (of possibly 0) to events. But I have not tried to implement it, so there is probably some hurdle when actually implementing it... The biggest problem I can see is that of choices. Choices usually stop auto_skip, we might need some functionality to automatically select a choice (possibly needing to detect which choices need to be selected to get to the requested point in the timeline 😟. This sounds complicated....

@firedfromlife
Copy link
Contributor Author

I would still like to implement text editor functionality for this, gotta figure out how to do that without redoing the entire right click that already exists for the code editor...

In regards to the jump stuff, choices being I think the best argument for why a pre-process step may not be the best idea.
The options for implementation all seem kinda... bad.

  1. Auto select option x
    • I feel like this is somehow the least bad, but for sure is not without issues.
  2. It still pauses at all branching points
    • This kinda defeats the point of play from here in the first place.
  3. Remove those from the timeline
    • This sounds wildly complicated, and also may remove necessary pre-processing steps depending on the timeline structure.

@firedfromlife
Copy link
Contributor Author

I have confirmed that the built in right-click context menus can not be edited, as of right now. There is an open PR to implement the feature in Godot though so 🤞.

@Invertex
Copy link
Contributor

Invertex commented Aug 25, 2024

For the handling of multi-choice paths, what if play-from-here was an editor-only-event that was stripped on build/play?

This would allow people to setup test points within the timeline to select and play from.
The event could then have an integer array in it that defines all the choice indices up until that point. This array could be validated on timeline change and input to ensure those numbers are within valid ranges, testing it by reading through only the choice events and following that flow of choice values (taking care to look out for jump-events in-between of course).

This could be a secondary option perhaps too, there could still be a right-click option that just selects the shortest choice path, and so users can choose what fits best for their scenario.

This play-from event could be made an externally callable function for the timeline as well, able to receive the integer array so gameplay functionality can also take advantage of this while ensuring the needed events and values get set along the way.

@firedfromlife
Copy link
Contributor Author

I am not opposed to looking into the implementation of an editor only event. Basically, if it is playing in the test_timeline_scene.gd, then check if we are calling "play from here". If we are, we could check to see if we have a choices array of some kind, if so, process through those.

This could just auto_skip like @Jowan-Spooner suggested previously.

@salianifo
Copy link
Contributor

I have an idea that may help with both the visual editor and text editor. What if instead of adding to the right click menu (or perhaps in addition to it), we made the "Play Timeline" button into a dropdown menu with the default option being "Play Timeline" but another option of "Play Timeline From Selected Event" (or something like that), then in the visual editor it would use the earliest selected event (or alert you if no event is selected), and in the text editor it would use the earliest selected line.

Also, regarding being able to have it run the events up to that point, personally I think that's more complicated than it's worth when you consider all the potential jump paths between those points. At that point you would be better off using a jump and skip (I feel similarly about having an editor only event for it). If such complicated functionality was to be desired or needed, perhaps it can be added in a later PR so we can have the simple functionality sooner rather than waiting for the whole thing. That's just my take on it though, so take it with a grain of salt. ^_^

@Invertex
Copy link
Contributor

Well, processing the actual events up to the "Play from here" point is rather important for a lot of games, as it's going to alter content that's on screen up to that point.

But I agree that it may be best to just implement a simple play-from-here option for now and have a more all-encompassing improvement to it later. Just making sure to implement it now in a way that will leave that open without having to change things around too much...

My suggestion would still be the editor-only node, which in the future could be update to allow for defining a list of choices to make up to that point. Or a separate panel for defining "Play from points" that you can then just click on to start from one.

@CakeVR
Copy link
Collaborator

CakeVR commented Nov 11, 2024

In what state is this PR now?

@firedfromlife
Copy link
Contributor Author

@CakeVR

In what state is this PR now?

As of right now, I haven't worked on this since the original PR.
I am currently working on .psd file parsing using GDScript... It has taken up a considerable amount of time recently.

Getting back to this is next on my todo list though.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants