Skip to content

add: TimerSystem. Timer-based event handling API.#65

Open
sagrig wants to merge 1 commit into
cfnptr:mainfrom
sagrig:timer_feature
Open

add: TimerSystem. Timer-based event handling API.#65
sagrig wants to merge 1 commit into
cfnptr:mainfrom
sagrig:timer_feature

Conversation

@sagrig

@sagrig sagrig commented May 4, 2025

Copy link
Copy Markdown

Implements #64

@sagrig

sagrig commented May 4, 2025

Copy link
Copy Markdown
Author

The current callback signature 'bool(void)' is a draft. Further commit should generalize it, considering:

  • callbacks may be invoked explicitly (i.e. outside TimerSystem::runEvents())
  • callbacks may accept arguments and return objects of various types

@cfnptr cfnptr self-requested a review May 5, 2025 06:49
* \file timer.hpp
* \author Sahak Grigoryan <sahakgrigoryan.am@gmail.com>
*
* \brief Class definition: TimerSystem

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

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

All code in the Garden project use Apache 2.0 license. Please add here license info, if you agree to use this license for your code.

Copyright [yyyy] [name of copyright owner]

Licensed under the Apache License, Version 2.0 (the "License");
you may not use this file except in compliance with the License.
You may obtain a copy of the License at

    http://www.apache.org/licenses/LICENSE-2.0

Unless required by applicable law or agreed to in writing, software
distributed under the License is distributed on an "AS IS" BASIS,
WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
See the License for the specific language governing permissions and
limitations under the License.

Comment thread include/garden/system/timer.hpp
/**
* \brief Timer metadata structure.
*/
struct TimerHandler {

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

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

In Entity Component System (ECS) paradigm this should be a TimerComponent, please see the ComponentSystem<> usage. With current implementation we can't attach this timer to an entity.

/**
* \brief Runs and manages all timer handlers; "Timer" ordered event handler.
*/
void runTimers(void);

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

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

This should be private, because it's subscribed to the "Timer" event which managed by the Manager.

/**
* \brief Wrapper method over memory pool releasing memory for handler objects.
*/
void disposeTimers(void)

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

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

All systems have built-in virtual disposeComponents() function, please see the ComponentsSystem<> for reference.

Comment thread source/system/timer.cpp
// ensure: (lowest supported time ratio) <= millisecond
static_assert(std::ratio_less_equal_v<ClockType::period, std::milli>);
ECSM_SUBSCRIBE_TO_EVENT("Timer", TimerSystem::runTimers);
ECSM_SUBSCRIBE_TO_EVENT("Timer", TimerSystem::disposeTimers);

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

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

Please see the ComponentsSystem<> disposeComponents() function for reference.

@cfnptr

cfnptr commented May 5, 2025

Copy link
Copy Markdown
Owner

The current callback signature 'bool(void)' is a draft. Further commit should generalize it, considering:

* callbacks may be invoked explicitly (i.e. outside TimerSystem::runEvents())

* callbacks may accept arguments and return objects of various types

I think we should use strings for timer function callback, like it's done in the PhysicsSystem for sensor collider events:

string eventListener; /**< Rigidbody events listener name. */

if (entity1)
{
auto rigidbodyView = getComponent(entity1);
toEventName(eventName, rigidbodyView->eventListener, bodyEvent.eventType);
thisBody = entity1; otherBody = entity2;
manager->tryRunEvent(eventName);
}

@cfnptr cfnptr added good first issue Good for newcomers feature A new feature development labels May 5, 2025
@cfnptr cfnptr linked an issue May 5, 2025 that may be closed by this pull request
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

feature A new feature development good first issue Good for newcomers

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Add TimerSystem for timer-based event handling

2 participants