4
\$\begingroup\$

Disclaimer: I just asked this on the regular SE, but was suggested to post it in here since it's kind of a code review.

I've recently been playing around with component frameworks, and I'm trying to create a basic trigger component. The component should be able to take one (or more) pair of values that can be compared against each other, triggering upon certain conditions.

The value pair should be of a generic type, I suggest an interface like IComparable. The TriggerComponent itself cannot be templated (i think?), as the component system must register it and would not be able to do so without supplying type information via angle brackets. Here's my current prototype version. It works, but some awkward issues that I'll explain below:

  /* interface IComparable.h */
  class IComparable
  {
  public:
    virtual ~IComparable() {}

    virtual bool lessOrEqual(const IComparable& other) const = 0;
  };

  /* Derived type container, intended for primitive values like int, float etc */
  template<class T>
  class NumberType : public IComparable
  {
  public:
    NumberType(const T& number) : m_number(number) {}
    bool lessOrEqual(const IComparable& other) const
    {
      NumberType& nt = (NumberType&)other;
      return m_number <= nt.m_number;
    }

    const T& m_number;
  };

  /* Component with threshold/trigger values made up by number types, but also objects of IComparable */
  class TriggerComponent : public artemis::Component
  {
  public:
    TriggerComponent(const IComparable& threshold, const IComparable& triggerValue)
      : m_thresholdValue(threshold), m_triggerValue(triggerValue) {}


    const IComparable& threshold() const { return m_thresholdValue; }
    const IComparable& triggerValue() const { return m_triggerValue; }

    void execute() const { qDebug() << "execute!"; }

  private:
    const IComparable& m_thresholdValue;
    const IComparable& m_triggerValue;
  };

The triggerValue parameter should be a reference as it is supposed to be monitored and checked against the threshold. Threshold should ideally be a copy, and not refer to anything. I would ideally like to have IComparable/NumberType as structs as it would make it easy to reference or copy as needed, but I don't think it is possible due to the pure interface.. As they're both reference values, I'm in a lot of cases forced to allocate heap memory, which i'd like to avoid for simple types.

If I had a cube in a 3D world with an ongoing scaling reduction, here's how I'd use the TriggerComponent to detect when it's X component reaches 1.0f:

    Entity* cubeEntity = /* omitted */
    ScaleComponent* scaleComp = /* omitted*/
    float* cubeScaleMinThreshold = new float(1.0f);
    NumberType<float>* threshold = new NumberType<float>(*cubeScaleMinThreshold);
    NumberType<float>* currScale = new NumberType<float>(scaleComp->getx());  
    cubeEntity .addComponent(new TriggerComponent(*threshold, *currScale));

I need the flexibility to create components on the fly, and I'd like to pass stack-created values as copies rather than passing raw pointer refs, but I'm not exactly sure how I can make it work in my current interface setup. I don't see how I can make generic comparisons between values (of same type) without one, but that seems to also require members to not be abstract value types.

I would be very appreciative on possible changes to make the design less insane. Maybe I'm just overlooking an obvious simple approach?

PS. I know raw pointers set up like I just did, leak like a 2 year old. It's to show why the design is terrible! :)

\$\endgroup\$
  • \$\begingroup\$ Welcome to Code Review! This does look like a good question for this site. I hope you get some good reviews! \$\endgroup\$ – Phrancis Nov 19 '14 at 17:40
0
\$\begingroup\$

Having thought a little bit on the problem, I've come up with a solution I think satisfy my goal without being too awkward or introducing gotchas with raw pointers and ownership. Below is my updated code and as always, I would appreciate any feedback or tips on improvement.

  class IComparable
  {
  public:
    virtual ~IComparable() {}
    virtual bool lessOrEqual() const = 0;
    virtual bool greaterOrEqual() const = 0;
    virtual bool equal() const = 0;
    virtual bool lessThan() const = 0;
    virtual bool greaterThan() const = 0;
  };

  /* trigger/threshold set of any type supporting the comparison operators. Can create new set by adhering to the interface*/
  template<class T>
  class ComparableTriggerSet : public IComparable
  {
  public:
    ComparableTriggerSet(std::shared_ptr<T> threshold, const T& trigger) : m_threshold(threshold), m_trigger(trigger) {}
    bool lessOrEqual() const { return m_trigger <= *m_threshold; }
    bool greaterOrEqual() const { return m_trigger >= *m_threshold; }
    bool equal() const { return m_trigger == *m_threshold; }
    bool lessThan() const { return m_trigger < *m_threshold; }
    bool greaterThan() const { return m_trigger > *m_threshold; }

  private:
    std::shared_ptr<T> m_threshold;
    const T& m_trigger;
  };

  /* supported operators */
  enum CompareOperator { LessOrEqual, GreaterOrEqual, Equal, LessThan, GreaterThan };

  /* TriggerComponent - getting checked at regular intervals by the TriggerSystem */
  class TriggerComponent : public artemis::Component
  {
  public:
    TriggerComponent(std::shared_ptr<IComparable> comparableTriggerSet, CompareOperator compareOperator)
      : m_comparableTriggerSet(comparableTriggerSet), m_compareOperator(compareOperator) {}


    void check() const
    {
      bool condition = false;
      switch(m_compareOperator)
      {
        case LessOrEqual:
          condition = m_comparableTriggerSet->lessOrEqual();
          break;
        case GreaterOrEqual:
          condition = m_comparableTriggerSet->greaterOrEqual();
          break;
        case Equal:
          condition = m_comparableTriggerSet->equal();
          break;
        case LessThan:
          condition = m_comparableTriggerSet->lessThan();
          break;
        case GreaterThan:
          condition = m_comparableTriggerSet->greaterThan();
          break;
        default:
          assert(false && "unsupported operator enum");
          break;
      }

      if(condition)
        execute();
    }

    /* supposed to invoke an event or maybe call a script or something similar */
    void execute() const
    {
      qDebug() << "execute!";
    }

  private:
    std::shared_ptr<IComparable> m_comparableTriggerSet;
    CompareOperator m_compareOperator;
  };

I would then add the component in my executable like so:

  artemis::Entity& ent = /* omitted */
  ScaleComponent* scaleComp = /* omitted */
  std::shared_ptr<float> threshold = std::make_shared<float>(1.0f);
  std::shared_ptr<ComparableTriggerSet<float>> triggerSet = std::make_shared<ComparableTriggerSet<float>>(threshold, scaleComp->getx());
  ent.addComponent(new TriggerComponent(triggerSet , CompareOperator::LessThan));

I think the updated code solves my original issues fairly well. I have thus far not encountered any problems with it, and I can add TriggerComponents without worry of whether threshold value is getting off the stack, cleaned up properly etc.

\$\endgroup\$

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.