ApplyEffect implementation #1688

heinezen · 2024-09-07T23:32:18Z

Implements a system to use the ApplyEffect ability of the nyan API.

Resolves #671
Depends on SFTtech/nyan#124
Depends on #1778

Other Features

Curves: Curves now support removing redundant keyframes (i.e. keyframes that can safely be deleted without changing any values on the curve). Every curve now has "compression" methods that detect and remove these keyframes. Compression can start at a specific simulation time, so that only the last X keyframes of a curve are compressed. We now use this compression in the renderer, so that animations are timed correctly. It will also be useful for networking to keep the curves small.
Renderer/Input: We now support targeting of other game entities using the ID texture created by the renderer (which writes game entity IDs to a texture). The controllers in the input system can use this texture to look up the ID of the game entity that was targeted.
Activities: Severa new conditions where added for applying effects, e.g. to check if a target is in range. There are also new node types for internal tasks and switching branches in the graph. Condition functions for XOR gates are now passed the nyan object that they are based on to allow giving more context for evaluating the condition.

paaspaas00 · 2025-03-16T17:16:12Z

Hey @heinezen , what's the state of this PR? Is it ready-ish or faraway? Maybe you can add some details in here

Copilot

Copilot reviewed 95 out of 96 changed files in this pull request and generated no comments.

Files not reviewed (1)

libopenage/curve/CMakeLists.txt: Language not supported

Comments suppressed due to low confidence (1)

libopenage/curve/continuous.h:77

The 'compress' parameter in set_insert is declared but not used; if compression is intended for insertions, consider incorporating this flag into the implementation or removing it to avoid confusion.

void Continuous<T>::set_insert(const time::time_t &t, const T &value, bool /* compress */) {

…bility.

…om input system.

…lity.

TheJJ

wow, this is really great 😄
just a few remarks and questions, but the generalized implementation of effect application looks truly well designed.

TheJJ · 2025-06-15T21:53:07Z

libopenage/curve/continuous.h

+template <KeyframeValueLike T>
+void Continuous<T>::set_insert(const time::time_t &t,
+                               const T &value,
+                               bool /* compress */) {
 	this->set_replace(t, value);


why not pass on compress?

TheJJ · 2025-06-15T21:59:07Z

libopenage/curve/interpolated.h

+	// Store elements that should be kept
+	std::vector<Keyframe<T>> to_keep;
+	auto last_kept = e;
+	for (auto current = e + 1; current < this->container.size() - 1; ++current) {


this has to be size - 2, otherwise the last element is doubled?

TheJJ · 2025-08-14T20:56:33Z

libopenage/curve/keyframe_container.h

 typename KeyframeContainer<T>::elem_ptr
 KeyframeContainer<T>::sync(const KeyframeContainer<T> &other,
                           const time::time_t &start) {
-	// Delete elements after start time
+	// Delete elements from this container after start time
 	elem_ptr at = this->last_before(start, this->container.size());


we could use the default last_before which uses the container size anyway.

TheJJ · 2025-08-14T21:03:44Z

libopenage/curve/keyframe_container.h

-			at = this->insert_after(other.get(i), at);
-		}
-	}
+	this->container.insert(this->container.end(),


this assumes sync always is done on future content only, maybe we should remember for a synced-to curve which point it synced at, so we can also handle changes that happened before the last sync.
is probably nasty to figure out later, but easy to at least error when such a sync happens as we see it now.

TheJJ · 2025-08-14T21:12:26Z

libopenage/gamestate/activity/condition/next_command_switch.h

+ * Check the type of the next command in the queue and return its associated key
+ * value.
+ *
+ * The key value is the result of a static_cast of the \p command_t enum value.


why not return the command_t value directly then?

TheJJ · 2025-08-14T21:34:45Z

libopenage/gamestate/api/definitions.h

+	std::pair("engine.util.command.type.Move",
+              component::command::command_t::MOVE),
+	std::pair("engine.util.command.type.ApplyEffect",
+              component::command::command_t::APPLY_EFFECT));


should we be able to define these lookup aliases within nyan somehow?

TheJJ · 2025-08-14T21:50:33Z

libopenage/gamestate/system/apply_effect.cpp

+	auto path_angle = path_vector.to_angle();
+
+	if (not turn_speed->is_infinite_positive()) {
+		auto angle_diff = path_angle - effector_angle;


std::abs?

TheJJ · 2025-08-14T22:03:40Z

libopenage/gamestate/system/apply_effect.cpp

+}
+
+
+const component::attribute_value_t ApplyEffect::get_applied_discrete_flac(const std::vector<nyan::Object> &effects,


s/flac/flat?

TheJJ · 2025-08-14T22:15:25Z

libopenage/gamestate/system/move.cpp

+	else if (std::holds_alternative<coord::phys3>(target.value())) {
+		auto target_pos = std::get<coord::phys3>(target.value());
+		return Move::move_default(entity, state, target_pos, start_time);
+	}


you can select the variant alternatives without a funny if block using std::visit like this:

using value_t = std::variant<int, long, double, std::string>; template<class... Ts> struct overloaded : Ts... { using Ts::operator()...; }; std::vector<value_t> vec = {10, 15l, 1.5, "hello"}; for (auto &v : vec) { std::visit(overloaded{ [](auto arg) { std::cout << arg << ' '; }, [](double arg) { std::cout << std::fixed << arg << ' '; }, [](const std::string& arg) { std::cout << std::quoted(arg) << ' '; } }, v); }

TheJJ · 2025-08-14T22:18:41Z

libopenage/input/controller/game/controller.cpp

+				{"target", target_entity_id},
+				{"entity_ids", controller->get_selected()},
+			};
+		}


this is just awesome

heinezen added nice new thing ☺ A new feature that was not there before lang: c++ Done in C++ code area: simulation Involved in the game mechanics and simulation big stuff High-impact changes, mainly foundation work labels Sep 7, 2024

heinezen force-pushed the feature/apply_effect branch 2 times, most recently from fe0e630 to 5b96a18 Compare September 8, 2024 19:59

heinezen force-pushed the feature/apply_effect branch from 03fd671 to 61d409b Compare October 5, 2024 19:32

heinezen force-pushed the feature/apply_effect branch from 8449c6d to 51416c3 Compare October 15, 2024 01:46

heinezen added the kevin-rebuild-pl0x instruct kevin to rebuild this pull request label Oct 20, 2024

SFTbot removed the kevin-rebuild-pl0x instruct kevin to rebuild this pull request label Oct 20, 2024

heinezen force-pushed the feature/apply_effect branch 3 times, most recently from d79f834 to 26bac42 Compare November 5, 2024 04:17

heinezen force-pushed the feature/apply_effect branch from 30c0ef0 to 35d4f37 Compare November 18, 2024 22:46

heinezen force-pushed the feature/apply_effect branch from 35d4f37 to 63bc382 Compare December 2, 2024 03:37

heinezen force-pushed the feature/apply_effect branch 2 times, most recently from b1ac0c6 to a4b8864 Compare March 30, 2025 16:53

heinezen added the kevin-rebuild-pl0x instruct kevin to rebuild this pull request label Mar 30, 2025

SFTbot removed the kevin-rebuild-pl0x instruct kevin to rebuild this pull request label Mar 30, 2025

heinezen mentioned this pull request Apr 8, 2025

Fix assets conversion bug. Add running with docker docs #1767

Merged

heinezen requested a review from Copilot April 16, 2025 22:54

Copilot AI reviewed Apr 16, 2025

View reviewed changes

heinezen force-pushed the feature/apply_effect branch 5 times, most recently from 6023b34 to 2f0e318 Compare April 22, 2025 00:43

heinezen force-pushed the feature/apply_effect branch from 635b0f5 to fd5ea13 Compare May 2, 2025 00:17

heinezen mentioned this pull request May 10, 2025

Allow retrieving nyan::Object handle as shared_ptr SFTtech/nyan#124

Merged

heinezen added 28 commits June 13, 2025 19:55

gamestate: Add condition for checking if a target is in range of an a…

467725f

…bility.

gamestate: Store target of entity when receiving events.

29a409a

convert: Add range check before apply effect ability in activity,

581325b

gamestate: Add API interface for checking generic API objects.

8227138

gamestate: More reliable method to find API parent of nyan object.

d6a4a77

gamestate: Add method for checking if game entity/player exists.

cb6d7f3

input: Fix type of target ID send in command.

754607c

gamestate: Make TargetInRange condition accessible for activity system.

1354cd2

gamestate: Remove unnecessary parameters from ApplyEffect system.

65da27b

convert: Add task nodes to nyan API loader.

484aa7d

convert: Add clear queue task to default activity.

ba79d39

curve: Fix breakout condition when clearing queue.

a776a65

gamestate: Use clear queue task in activity system.

ea4cda9

gamestate: Make apply effect system use command information.

6fc81d7

gamestate: Remove separate queue for command queue targets.

730b7ab

gamestate: Add system for moving to target.

77f5516

gamestate: Private default movement to deestination.

5b212aa

convert: Move to target in default unit activity.

f0cd440

gamestate: Handle MoveToTarget task activity type.

032ce8a

gamestate: Fix wrong log messages.

9863658

gamestate: Clear command queue by default when new command is sent fr…

82dc155

…om input system.

convert: Add activity node for checking if the game entity has an abi…

fbe05ad

…lity.

gamestate: Add condition function for AbilityUsable check.

782a152

gamestate: Turn entity towards target before applying effect.

fd7d4ce

doc: Move system descriptions into code docs.

5f45145

doc: Move component descriptions into code docs.

0b3297b

cfg: Update modpack version of converted games.

996c77d

convert: Fix semantic version comparison.

6dc3ca7

heinezen force-pushed the feature/apply_effect branch from be52171 to 6dc3ca7 Compare June 13, 2025 17:57

TheJJ reviewed Aug 15, 2025

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

Uh oh!

ApplyEffect implementation #1688

ApplyEffect implementation #1688

Uh oh!

heinezen commented Sep 7, 2024 •

edited

Loading

Uh oh!

paaspaas00 commented Mar 16, 2025

Uh oh!

Copilot AI left a comment

Uh oh!

TheJJ left a comment

Uh oh!

TheJJ Jun 15, 2025

Uh oh!

TheJJ Jun 15, 2025

Uh oh!

TheJJ Aug 14, 2025

Uh oh!

TheJJ Aug 14, 2025

Uh oh!

TheJJ Aug 14, 2025

Uh oh!

TheJJ Aug 14, 2025

Uh oh!

TheJJ Aug 14, 2025

Uh oh!

TheJJ Aug 14, 2025

Uh oh!

TheJJ Aug 14, 2025

Uh oh!

TheJJ Aug 14, 2025

Uh oh!

Uh oh!

		}


		const component::attribute_value_t ApplyEffect::get_applied_discrete_flac(const std::vector<nyan::Object> &effects,

Uh oh!

ApplyEffect implementation #1688

Are you sure you want to change the base?

ApplyEffect implementation #1688

Uh oh!

Conversation

heinezen commented Sep 7, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Other Features

Uh oh!

paaspaas00 commented Mar 16, 2025

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Uh oh!

TheJJ left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

heinezen commented Sep 7, 2024 •

edited

Loading