Skip to content

Conversation

@sagrig
Copy link

@sagrig sagrig commented May 4, 2025

Implements #64

@sagrig
Copy link
Author

sagrig commented May 4, 2025

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
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.

/**
* \brief Timer metadata structure.
*/
struct TimerHandler {
Copy link
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
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
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.

// 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
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
Copy link
Owner

cfnptr commented May 5, 2025

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.

(System) Add TimerSystem for timer-based event handling

2 participants