Skip to content

Switch from player modes to simultaneous player abilities#2032

Open
manuq wants to merge 38 commits intomainfrom
player-abilities
Open

Switch from player modes to simultaneous player abilities#2032
manuq wants to merge 38 commits intomainfrom
player-abilities

Conversation

@manuq
Copy link
Collaborator

@manuq manuq commented Mar 6, 2026

Context

Prior to this change, the player actions (besides walking, running) depended of which mode the Player instance was:

  • "Cozy" let the player interact with NPCs and other elements. For example: start a dialogue, collect a magical thread.
  • "Fighting" let the player repel projectiles.
  • "Hooking" let the player use a grappling hook.
  • "Defeated" forbid any action.
Grabacion.de.pantalla.desde.2026-03-11.14-45-39.mp4

On one hand this was simple enough, and allowed quests to have levels that focused on a single mechanic. But on the other hand it was too limiting:

  • The player needed to switch from one mode to the other in the same level. For example, to collect the magical thread after an ink fight.
  • The actions can't be conbined.

It is also confusing to use the same input mapping for different actions during a play run.

Most importantly, modes don't allow player progression. We want the player to gain these abilities during the game. And maybe enhace (even temporarily debuff) these abilities during the game.

The abilities could be gained in different order depending on the player decisions. For example, getting involved in a quest before another. This allows some replayability and non-linearity to the otherwise linear quests.

On the learning / user-contributed content side, a StoryQuest (considering it an isolated narrative unit or minigame) can also have a similar player progression as the main game.

Changes for the main game

  • Add flags for abilities and ability modifiers to the game state.
  • Remove player modes. Instead, check the game state to turn on/off mechanics ("repel", "grapple") and mechanic changes (longer thread powerup for "grapple").
  • The player starts the game without abilities.
  • Grant "repel" in Musician quest, at the beginning of ink_combat_round_1 (2nd) level.
  • Grant "grapple" in the Void quest, at the beginning of grappling_hook_start (2nd) level.
  • Grant "longer thread" modifier for "grapple", in the Void quest, when collecting the powerup item in grappling_hook_powerup (5th) level.
  • Grant "longer thread" modifier for "grapple" too in the same Void quest void_grappling_round_2 last level. Although it should be already given at this point, just to avoid redesigning the levels.

Note: we want these abilities to be powerups that the player collects, not granted like in here. But that will be addressed separately.

Grabacion.de.pantalla.desde.2026-03-11.00-25-01.mp4

Changes for StoryQuests

Change for learners: The Player doesn’t have a “mode” to set per-level anymore. The StoryQuests can have player progress through the levels by gaining abilities (“repel”, “grapple”). This could be granted by the level or by collecting an item (in the future, but will be the desired option). Learners could potentially create their own player abilities.

To help with transitioning StoryQuests, there is a new PlayerMode node. This holds the same options as before "COZY/FIGHTING/HOOKING" and will set a single ability ("repel" for fighting, "grapple" for hooking) and discard all abilities gained in previous levels. New StoryQuest creators are ancouraged to add player progress to their stories instead of using this node.

Saved state / Difference between World Map, Lore quests and StoryQuests

There are two saved states. Player abilities are saved globally for the whole game run, and also for the current quest. How this happens depends of the quest being a lore quest or a StoryQuest.

  • When a quest is resumed (lore or SQ), the abilities already obtained (or potentially lost) during this quest should be reflected.

For lore quests:

  • Lore quests start with a copy of the global abilities.
  • When a lore quest is abandoned, the abilities in World Map are the same as before starting. Any obtained (or lost) ability during the abandoned quest are dropped.
  • When a lore quest is completed, the global abilities are updated / copied from the current quest. So any ability obatained (or lost) are reflected in the full game.

For StoryQuests:

  • They start with no abilities at all (see PlayerMode node above for how the existing ones are being transitioned).
  • When a StoryQuest is abandoned or completed, the abilities in World Map are the same as before starting.

New input controls mapping

For keyboard + mouse (best experience):

  • WASD for moving
  • Shift for sprint
  • Space bar to interact
  • Mouse move for aiming
  • Left click to grapple
  • Right click to repel (new)

For just keyboard:

  • Arrows for moving + aiming
  • Shift for sprint
  • Z to grapple (new)
  • X to repel (new)
  • Space bar to interact

For joypad (let's say XBox):

  • Left stick or D-pad for moving
  • Right stick for aiming
  • LB for sprint
  • RB to repel (new)
  • RT to grapple
  • A to interact

Example gameplay

  • Start the game normally. There should be no repel or grapple abilities.
  • Get involved in The Void quest. Reach level 2. The player is now able to grapple.
  • Abandon the quest. No grapple anymore.
  • Get involved in The Void quest again. This time complete it. The player is now able to grapple in the world map too!
  • Get involved in Stella StoryQuest. The player starts with no abilities in 1st (stealth) level.
  • Reach Stella level 2 (combat), the player should be able to repel.
  • Abandon quest. The player should have the grapple ability.
  • Get involved in The Musician quest. The player should be able to grapple during the 1st (sequence puzzle) level.
  • Reach level 2 (combat). The player should be able to repel + grapple.
  • Complete the quest. The player should now be able to repel + grapple in the world map.

Resolve #1375

@github-actions
Copy link

github-actions bot commented Mar 6, 2026

Play this branch at https://play.threadbare.game/branches/endlessm/player-abilities/.

(This launches the game from the start, not directly at the change(s) in this pull request.)

@manuq
Copy link
Collaborator Author

manuq commented Mar 6, 2026

For now the draft ignores player modes at all (except for "defeating"), and makes the 2 abilities "repel" and "grapple" active all the time. Interact is not considered an ability.

recording.webm

I did a few changes in the input mapping. But I really welcome alternatives:

For keyboard + mouse:

  • WASD for moving
  • Shift for sprint
  • Space bar to interact
  • Mouse move for aiming
  • Left click to grapple
  • Right click to repel (new)

For just keyboard:

  • Arrows for moving + aiming
  • Shift for sprint
  • Z to grapple (new)
  • X to repel (new)
  • Space bar to interact

For joypad (let's say XBox):

  • Left stick or D-pad for moving
  • Right stick for aiming
  • LB for sprint
  • RT to grapple
  • B to repel (new)
  • A to interact

(While doing this I noticed that the Champ quest introduced input maps to the project).

What about the mouse cursor? Should it always be a crosshair except while interacting with dialogue or the game menu?

We should think about: How would this affect the lore quests and the StoryQuests? Some ideas:

  • "Repel" and "grapple" abilities should be part of the player progression and persisted in game state.
  • But StoryQuests should not obey the lore progression and Player instances in SQs.
  • Ideally each SQ has their own copy of player.tscn (like the existing NO_EDIT_projectile.tscn) and could even have their own custom abilities.

Some other changes needed:

  • Replace the existing player modes. Maybe think in new ones? For the sake of input handling, at least player defeated is a mode.
  • When the player goes through the grappling line, the PlayerHook takes control over the player walking. This is a bit hacky and could be refactored. Maybe another mode?
  • I noticed that PlayerRepel also handles player harm for projectiles. This should be rearranged.
  • A StoryQuest "After the Tremor" was using player scripts that I refactored here. Is probably broken.
  • The input hints will need to be updated.

@wjt
Copy link
Member

wjt commented Mar 9, 2026

For now the draft ignores player modes at all (except for "defeating"), and makes the 2 abilities "repel" and "grapple" active all the time. Interact is not considered an ability.

recording.webm
I did a few changes in the input mapping. But I really welcome alternatives:

For keyboard + mouse:

  • WASD for moving
  • Shift for sprint
  • Space bar to interact
  • Mouse move for aiming
  • Left click to grapple
  • Right click to repel (new)

Great combination of actions.

For joypad (let's say XBox):

  • Left stick or D-pad for moving
  • Right stick for aiming
  • LB for sprint
  • RT to grapple
  • B to repel (new)
  • A to interact

I tested these - it's great to have the combination of actions available at the same time!

It is very hard to repel while also aiming the grappling hook with the right stick because the same thumb is used for both. We could perhaps move the repel action to a shoulder button.

I was trying to think of bindings that I thought were good. It's a bit hard to google for the bindings for the game "control" for obvious reasons but they're here. In that game the right stick moves the camera, and the left stick moves the character - sort of similar. They use:

  • Sprint: Push the left stick (we use LB)
  • Shield (a defensive action like repel): LB
  • Dodge (another defensive action): B

But this is something we don't have to get right first time...

(While doing this I noticed that the Champ quest introduced input maps to the project).

This seemed the simplest way they could do what @jgbourque was asking for: a custom player script with gaps for multiple new special abilities. If we had had the changes you're drafting here with 4 actions with distinct bindings, maybe champ could have instead have (ab)used those, e.g. use the grapple action for the teleport special ability.

What about the mouse cursor? Should it always be a crosshair except while interacting with dialogue or the game menu?

I would say:

  • Pointer when in the game menu or when the cursor is moved during dialogue
  • Crosshair when grappling is available
  • Otherwise hidden
  • "Repel" and "grapple" abilities should be part of the player progression and persisted in game state.
  • But StoryQuests should not obey the lore progression and Player instances in SQs.

Great point. And you could imagine a storyquest where the player-character gains one of these abilities during the course of the storyquest... So the persistence/restoring of these abilities needs to be factored out in some way.

  • Ideally each SQ has their own copy of player.tscn (like the existing NO_EDIT_projectile.tscn) and could even have their own custom abilities.

I think ultimately we will need this but ironically the reason I was starting small with the combat projectile scene, and the sequence puzzle object scene, is that those scenes are unlikely to change dramatically & need updating in every storyquest, unlike the player scene where we knew that exactly this refactoring would be needed!

@manuq manuq force-pushed the player-abilities branch 2 times, most recently from 5ff7cf4 to e0ce806 Compare March 9, 2026 21:53
@manuq
Copy link
Collaborator Author

manuq commented Mar 9, 2026

It is very hard to repel while also aiming the grappling hook with the right stick because the same thumb is used for both. We could perhaps move the repel action to a shoulder button.

Good idea! I have updated it to be the right shoulder button. I added the input mapping to the PR description. I also started a wiki page in which we can document this. I wonder since we are not targetting consoles if the joypad experience for aiming can be left as-is for now? And we consider mouse + keyboard the best experience? Up for discussion!

I was trying to think of bindings that I thought were good. It's a bit hard to google for the bindings for the game "control" for obvious reasons but they're here. In that game the right stick moves the camera, and the left stick moves the character - sort of similar. They use:

  • Sprint: Push the left stick (we use LB)
  • Shield (a defensive action like repel): LB
  • Dodge (another defensive action): B

But this is something we don't have to get right first time...

Interesting! I think is worth trying moving sprint to "push left stick" and LB for the repel action.

(While doing this I noticed that the Champ quest introduced input maps to the project).

This seemed the simplest way they could do what @jgbourque was asking for: a custom player script with gaps for multiple new special abilities. If we had had the changes you're drafting here with 4 actions with distinct bindings, maybe champ could have instead have (ab)used those, e.g. use the grapple action for the teleport special ability.

OK!

What about the mouse cursor? Should it always be a crosshair except while interacting with dialogue or the game menu?

I would say:

  • Pointer when in the game menu or when the cursor is moved during dialogue
  • Crosshair when grappling is available
  • Otherwise hidden

I agree, I started doing so in the last 3 commits but I'm not sure I'm doing it in the right places. I did:

  • Pause overlay: Set cursor to arrow and back to cross when paused/unpaused
  • Dialogue balloon: Set cursor to arrow and back to cross when started/ended
  • Mouse manager: Set cursor to cross by default

I'm still thinking about the rest of your feedback (player progression). Thanks!

@wjt
Copy link
Member

wjt commented Mar 10, 2026

Interesting! I think is worth trying moving sprint to "push left stick" and LB for the repel action.

I think this also gets back to our "who is the player" question. In my experience trying to teach people how to play Katamari Damacy (which is controlled basically entirely with the two sticks), the "click both joysticks to do a 180° turn" control is really surprising to people at first: less experienced gamers don't see the joysticks as being buttons as well as joysticks. But you only have to teach it once.

@manuq manuq force-pushed the player-abilities branch from dc58824 to cb13a7c Compare March 11, 2026 03:25
@manuq
Copy link
Collaborator Author

manuq commented Mar 11, 2026

Update, the last 4 commits add player abilities to the game state, and use them in the lore game. There are 2 sets of flags:

  • Abilities for the main "lore" game. They last the entire game.
  • Abilities for the current StoryQuest. To allow player progression in that storytelling unit.

The same player scene calls set_ability(), has_ability() for the 2 cases, which makes the script complicated. But that's what we have for now.

Grabacion.de.pantalla.desde.2026-03-11.00-25-01.mp4

After a quick lore playthrough, is nice to obtain the repel ability or the grapple ability in different order depending which quest I play first. Adds a bit of non-linearity. Is also fun to replay The Void, and in the second level be able to obtain the thread directly because I already got the longer thread the first time I completed the quest :D

recording.webm

There is refactor to do. For instance, the grappling_hook_start and grappling_hook_end scenes can finally become one! (if I figure out how to start/skip the cinematic depending on the spawning point).

Caveat: you can abandon the (lore) quest right after obtaining the ability and you keep it! I think it should be finally granted when completing the quest.

Caveat: I added 100 more lines to the already bulky game_state.gd.

Note: this breaks all StoryQuest scenes that set the old player mode to grapple or repel. I hope to fix them soon by granting/removing the abilities per scene.. yikes.

@wjt
Copy link
Member

wjt commented Mar 11, 2026

There is refactor to do. For instance, the grappling_hook_start and grappling_hook_end scenes can finally become one! (if I figure out how to start/skip the cinematic depending on the spawning point).

Put an Area2D around both spawn points; if the first one triggers, start the cinematic then disable the area2d; if the second one triggers, disable the first area2d?

@manuq
Copy link
Collaborator Author

manuq commented Mar 11, 2026

There is refactor to do. For instance, the grappling_hook_start and grappling_hook_end scenes can finally become one! (if I figure out how to start/skip the cinematic depending on the spawning point).

Put an Area2D around both spawn points; if the first one triggers, start the cinematic then disable the area2d; if the second one triggers, disable the first area2d?

I'll try this now!

@manuq manuq changed the title Player abilities Switch from player modes to player abilities Mar 11, 2026
@manuq manuq changed the title Switch from player modes to player abilities Switch from player modes to simultaneous player abilities Mar 11, 2026
@manuq manuq force-pushed the player-abilities branch from cb13a7c to 7fde536 Compare March 12, 2026 19:24
@manuq
Copy link
Collaborator Author

manuq commented Mar 12, 2026

A drawback of player abilities (compared with player modes) is that while developing we usually run a specific scene, or ask someone to playtest a specific scene (giving a URL hash). In those cases game progress is ignored (not loaded or saved). So this PR started without any abilities. Although they can be enabled from the Inspector while the game is running, is it tedious to do it each time. So in the last 2 commits I:

  • Made repel and grapple (but not long thread grapple) be granted when running a specific scene / without progress
  • Also asked Claude to create UI for it in the Debug Settings. This is for URL hash scenes in web builds.

@manuq
Copy link
Collaborator Author

manuq commented Mar 13, 2026

There is refactor to do. For instance, the grappling_hook_start and grappling_hook_end scenes can finally become one! (if I figure out how to start/skip the cinematic depending on the spawning point).

Put an Area2D around both spawn points; if the first one triggers, start the cinematic then disable the area2d; if the second one triggers, disable the first area2d?

This worked with some tweaks: I had to disable monitoring in the Area2D around the default player position and enable it after a small delay, because the player was entering both areas (before and after the teleporter did the job).

Then I tried removing that delay by adding a new signal to the SpawnPoint:

@tool
class_name SpawnPoint
extends Marker2D

## Emitted after the player position has been changed.
signal player_teleported

Then I noticed that the signal was being emitted before the whole level was ready. So I didn't need the Area2Ds anymore. I can just have an inner variable:

var _play_cinematic: bool = true 

func _ready() -> void:
	if _play_cinematic:
		cinematic.start()

func _on_spawn_point_from_powerup_player_teleported() -> void:
	# The player is coming back from the east, so play the
	# animation directly. This is actually letting the Void
	# add tiles.
	_play_cinematic = false
	if not is_node_ready():
		await ready
	void_patrolling.visible = false
	animation_player.play(&"eat_floor")

I added a way to start the cinematic manually too, with an autostart export that is true by default.

@manuq
Copy link
Collaborator Author

manuq commented Mar 13, 2026

I have now adjusted the input hints to the new mapping, including hints for mouse motion and clicks. We can of course change this in the future.

Captura desde 2026-03-13 16-47-58 Captura desde 2026-03-13 16-48-08 Captura desde 2026-03-13 16-48-14

@manuq manuq force-pushed the player-abilities branch from afb0898 to c186ae2 Compare March 16, 2026 18:47
@manuq manuq requested a review from a team March 16, 2026 18:48
@manuq
Copy link
Collaborator Author

manuq commented Mar 16, 2026

Update: I have fixed all StoryQuests, adding a new node to help with the "modes to abilities" transition. See section "Changes for StoryQuests" in the PR description. I did it with help from Claude, which created this Python script.

I also opened followup tickets because this change is becoming way to big:

I think what's missing is:

  • Completely remove player mode, or reinstate it to be:
    • "playing" - normal playing, player input can walk/run and use abilities
    • "controlled" - the player is waiting idle until dialogue completes or being moved by other means: pulled by the grappling, put on rails maybe? Doesn't attend to input.
    • "defeated" - the player displays the defeating animation and as the previous mode, doesn't attend to input. Or are these two last modes the same? Hmm.
  • Save global abilities only after completing the lore quest.

@manuq manuq force-pushed the player-abilities branch from 020a1f3 to 999625a Compare March 17, 2026 20:11
manuq added 6 commits March 17, 2026 17:46
Let them happen all at once without changing the actual modes for now.
Only leave the mode "defeating" to disable player interaction and
abilites (repel, grapple). This prevents refactoring the modes right
now.
This was introduced a while ago and left invisible. It has the shape of
StoryWeaver so it won't work when replacing the SpriteFrames in
StoryQuests.
This could be a const, or the "repel" string can be used directly.

This is only preparation for upcoming animations refactor.

Note: There are references in StoryQuest "After the Tremor" that
this change may break.
Rename the node that provides this ability to PlayerRepel. Adjust the
scripts accordingly.
And the Area2D nodes that had this script. In 2 scenes from Void quest.
Add a signal to the repel ability and use it to play the animation when
the property changes, instead of checking it constantly in _process().

Loop the repel animation in the script by adding a handler to the
animation finished signal, and remove the loop mode from the animation
itself.

Give a class name to PlayerRepel.
manuq added 6 commits March 18, 2026 17:42
The after_the_tremor StoryQuest extended the script to customize it. So upstream that change.
Use the new action input export of PlayerRepel to change it.
Disable the ability after the boss is defeated with the PlayerMode
node.

Also remove moving the PlayerInteraction inner node to a far away
position, and then back after the boss is defeated. There doesn't
seem to be a need for this.
Change the modes to be:
- "playing" for the usual case in which the player is controlled by the user.
- "controlled" for cases in which the game takes control of the player: when interacting, pulling the grappling hook and other potential on-rails cases.
- "defeated" as a clear state in which the player can't be controlled and won't be controlled anymore by the user.

Change PlayerHook to take control over the player. Also change PlayerInteraction to take control over the player. When a dialogue is shown it already takes user input, but the player might be interacting with other things.
This seems to be the last direct reference to Player.Mode.
mode has been repurposed, and scenes that instantiate Player shouldn't be setting it anymore.
Change global and quest player abilities:

- If a SQ is started, global abilities are not loaded into the quest
- If a lore quest is started, global abilities are loaded into the quest
- If a SQ is completed, global abilities are loaded to the world scenes
- If a lore quest is completed, abilities gained in quest are saved to global state (use bitwise to only add? or also let lost?)
@manuq manuq force-pushed the player-abilities branch from 999625a to 6b94e85 Compare March 18, 2026 20:42
@manuq manuq marked this pull request as ready for review March 18, 2026 20:43
@manuq manuq requested review from a team as code owners March 18, 2026 20:43
@manuq
Copy link
Collaborator Author

manuq commented Mar 18, 2026

Update: This is now ready for review. It is huge as the transitioning of SQ levels and removing Player.mode touches almost every single scene that has a Player instance.

I think what's missing is:

  • Completely remove player mode, or reinstate it to be:

    • "playing" - normal playing, player input can walk/run and use abilities
    • "controlled" - the player is waiting idle until dialogue completes or being moved by other means: pulled by the grappling, put on rails maybe? Doesn't attend to input.
    • "defeated" - the player displays the defeating animation and as the previous mode, doesn't attend to input. Or are these two last modes the same? Hmm.

I did this. and it makes it simple now to use the InputWalkBehavior in the player. But I left that as a comment and I will work on it after this is merged. Because this change became huge.

  • Save global abilities only after completing the lore quest.

I did this too. See section in description "Saved state / Difference between World Map, Lore quests and StoryQuests"

@manuq
Copy link
Collaborator Author

manuq commented Mar 18, 2026

I have written Example gameplay section in the description, and already found a bug. Checking!

@manuq
Copy link
Collaborator Author

manuq commented Mar 18, 2026

I have written Example gameplay section in the description, and already found a bug. Checking!

Fixed in the last fixup commit. It was a silly thing.

pulling = false
# After pulling, return control to the user.
if character.has_method("return_control"):
character.return_control(self)
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

The alternative to this I was handling is using the mode directly:

if character is Player:
	# Return control to the user:
	(character as Player).mode = Player.Mode.PLAYING

I guess duck-typing is better so a learner can do their own Player scenes from scratch, not necessarily extending the Player class.

Comment on lines +352 to +355
if _use_global_abilities():
game_player_abilities |= ability
else:
quest_player_abilities |= ability
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

This seems over-engineered. Could be simplified if we design the game in a way that abilities can be obtained only during lore quests.

PLAYING,
## Player is being controlled by other means: interacting,
## pulling the grappling hook, being put on rails, etc.
CONTROLLED,
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I hope this CONTROLLED enum is not misunderstood as "controlled by the player/user". It is also confusing that "player" can be the user playing or the player character (common issue in gamedev). If so here is an alternative option:

  • PLAYER_CONTROLLED / USER_CONTROLLED # Player is reacting to user input.
  • SYSTEM_CONTROLLED # Player is being controlled by other means: interacting, pulling the grappling hook, being put on rails, etc.
  • DEFEATED

Copy link
Member

Choose a reason for hiding this comment

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

I agree that the name is a bit confusing. I slightly prefer:

  • USER_CONTROLLED (because we named this script Player not PlayerCharacter -defensible, it's so much shorter!)
  • SYSTEM_CONTROLLED
  • DEFEATED

More verbose, but unambiguous.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Great I'll do the rename in a fixup.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I'm not happy with having 2 thin scrolling text areas:

Image

But that was the easiest to do. In the future we could have tabs in the Debug Settings, maybe?

Copy link
Member

Choose a reason for hiding this comment

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

Another option would be to have just one ScrollContainer, enclosing both lists of options. But I think tabs are likely better. Let's leave this for now and come back to it.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

OK! Yes I don't think we have Threadbare theme that supports tabs, I'll check.

Comment on lines +55 to +59
# Repel animation is being played for the first time. So skip the anticipation and go
# directly to the action.
speed_scale = original_speed_scale
play(&"repel")
seek(REPEL_ANTICIPATION_TIME, false, false)
Copy link
Member

Choose a reason for hiding this comment

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

It's not new in this PR but it's a little weird how this animation "starts" in the middle! I can't quite figure out why it is that way around. (No need to change it.)

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

This really needs some explanation. The animation we got from PixelFrog has 1 frame of anticipation, which is great. From Wikipedia:

For example, a baseball pitcher must "wind up" before throwing the ball. This is called the anticipation for the main action.

But in videogames the controls feel slow if the action doesn't happen immediately. So the idea is that the action is immediate (starts without anticipation) when the user press the input, but subsequent ones do display the anticipation, to also space the repeaded actions.

I've seen this in videogames, let me come up with a reference.

Comment on lines +101 to +110
func _validate_property(property: Dictionary) -> void:
match property["name"]:
# Treat the player abilities as bit flags.
# The @export_flags would be ideal but it expects constant
# strings, and we want to use the PlayerAbilities enum keys
# as hint strings.
# This also requires this script to be a @tool.
"lore_player_abilities", "storyquest_player_abilities":
property.hint = PROPERTY_HINT_FLAGS
property.hint_string = ",".join(Enums.PlayerAbilities.keys())
Copy link
Member

Choose a reason for hiding this comment

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

I spent some time trying to find an alternative to this (as I would guess you did too) but could not.

I did find this:

PROPERTY_USAGE_CLASS_IS_BITFIELD = 512

I wonder if that can be used somehow. (Not a blocker.)

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Yes I dedicated a while to try different things. I haven't seen a way to avoid hardcoding the strings (which on the other side are generic names, not "repel"). I can also open an issue upstream and first check if there is one.

Comment on lines +18 to +20
# The player is coming back from the east, so play the
# animation directly. This is actually letting the Void
# add tiles.
Copy link
Member

Choose a reason for hiding this comment

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

Another approach here would be to have a more general way to save state in scenes, then the enemy could remember its position and the Void layer could remember what has been eaten... I think this is fine for now.

Copy link
Member

@wjt wjt left a comment

Choose a reason for hiding this comment

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

Well done for pushing through this huge refactoring!

The part I'm least sure about is how the ability state is managed & synced between the player scene and the save file. I wonder if a simpler design would be possible if we used a different player scene in lore quests compared to in StoryQuests. The different scenes could then manage abilities in game state (or not) in different ways. However that would still leave the part where we need to track abilities that have been earned in previous completed quests, versus abilities that have been earned in the current quest but would be lost if the quest were abandoned. (And then, if we allow resuming an abandoned quest from the point where it was left off, restored!) Perhaps this is something we can explore further later.

I have some notes below but I don't think anything is a blocker here.

Copy link
Member

Choose a reason for hiding this comment

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

Normally we would name this file player_mode.gd.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Oh good catch! I'll fix it.

PLAYING,
## Player is being controlled by other means: interacting,
## pulling the grappling hook, being put on rails, etc.
CONTROLLED,
Copy link
Member

Choose a reason for hiding this comment

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

I agree that the name is a bit confusing. I slightly prefer:

  • USER_CONTROLLED (because we named this script Player not PlayerCharacter -defensible, it's so much shorter!)
  • SYSTEM_CONTROLLED
  • DEFEATED

More verbose, but unambiguous.

Comment on lines -184 to +179
if player_interaction.is_interacting or mode == Mode.DEFEATED:
velocity = Vector2.ZERO
# TODO: Use InputWalkBehavior instead, and enable/disable it when the mode changes.
if mode in [Mode.CONTROLLED, Mode.DEFEATED]:
Copy link
Member

Choose a reason for hiding this comment

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

This is a nice change. Previously if you trigger dialogue by means other than an interaction while the player is moving, then they stop responding to input but keep their velocity, so would keep walking across the screen! IIRC I ran into this while trying to fix #1347

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Yes at one point I said "wait, the dialogue captures the input, no need to system-control it". But then I saw StoryWeaver continue walking haha.

Copy link
Member

Choose a reason for hiding this comment

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

That's a downside of handling the input in _unhandled_input!

Copy link
Member

Choose a reason for hiding this comment

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

And as I discovered in #806 (comment) if we want to use 4.7's VirtualJoystick then we'll have to move the input handling back to _physics_process.

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

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Revisit player modes

2 participants