From b306c76cd8f9a7844cc0363e148be27e7f7ad205 Mon Sep 17 00:00:00 2001 From: Jeremiah Morgan Date: Tue, 3 Dec 2024 00:54:52 +0000 Subject: [PATCH 1/6] curve: common element wrapper --- libopenage/curve/element_wrapper.h | 54 ++++++++++++++++++++++++++ libopenage/curve/map.h | 22 +++-------- libopenage/curve/map_filter_iterator.h | 6 +-- libopenage/curve/queue.h | 33 ++-------------- 4 files changed, 66 insertions(+), 49 deletions(-) create mode 100644 libopenage/curve/element_wrapper.h diff --git a/libopenage/curve/element_wrapper.h b/libopenage/curve/element_wrapper.h new file mode 100644 index 0000000000..79756f0e5c --- /dev/null +++ b/libopenage/curve/element_wrapper.h @@ -0,0 +1,54 @@ +// Copyright 2024-2024 the openage authors. See copying.md for legal info. + +#pragma once + +#include "time/time.h" + +namespace openage::curve { + +/** + * wrapper class for elements of a curve, store the insertion time and erase time of the element + * aswell the data of the element. + */ +template +struct element_wrapper { + // Insertion time of the element + time::time_t _alive; + // Erase time of the element + time::time_t _dead; + // Element value + T value; + + /** + * construct new element, set its start time and value + * its end time will be set to time::TIME_MAX + */ + element_wrapper(const time::time_t &time, const T &value) : + _alive{time}, + _dead{time::TIME_MAX}, + value{value} {} + + // construct new element, set its start time, end time and value + element_wrapper(const T &value, const time::time_t &alive, const time::time_t &dead) : + _alive{alive}, + _dead{dead}, + value{value} {} + + + // start time of this element + const time::time_t &alive() const { + return _alive; + } + + // end time of this element + const time::time_t &dead() const { + return _dead; + } + + // set the end time of this element + void set_dead(const time::time_t &time) { + _dead = time; + } +}; + +} // namespace openage::curve diff --git a/libopenage/curve/map.h b/libopenage/curve/map.h index 9712c89470..d996095d7c 100644 --- a/libopenage/curve/map.h +++ b/libopenage/curve/map.h @@ -1,4 +1,4 @@ -// Copyright 2017-2023 the openage authors. See copying.md for legal info. +// Copyright 2017-2024 the openage authors. See copying.md for legal info. #pragma once @@ -8,6 +8,7 @@ #include #include "curve/map_filter_iterator.h" +#include "curve/element_wrapper.h" #include "time/time.h" #include "util/fixed_point.h" @@ -20,26 +21,15 @@ namespace openage::curve { */ template class UnorderedMap { - /** Internal container to access all data and metadata */ - struct map_element { - map_element(const val_t &v, const time::time_t &a, const time::time_t &d) : - value(v), - alive(a), - dead(d) {} - - val_t value; - time::time_t alive; - time::time_t dead; - }; /** * Data holder. Maps keys to map elements. * Map elements themselves store when they are valid. */ - std::unordered_map container; + std::unordered_map> container; public: - using const_iterator = typename std::unordered_map::const_iterator; + using const_iterator = typename std::unordered_map>::const_iterator; std::optional> operator()(const time::time_t &, const key_t &) const; @@ -95,7 +85,7 @@ std::optional>> UnorderedMap::at(const time::time_t &time, const key_t &key) const { auto e = this->container.find(key); - if (e != this->container.end() and e->second.alive <= time and e->second.dead > time) { + if (e != this->container.end() and e->second.alive() <= time and e->second.dead() > time) { return MapFilterIterator>( e, this, @@ -160,7 +150,7 @@ UnorderedMap::insert(const time::time_t &alive, const time::time_t &dead, const key_t &key, const val_t &value) { - map_element e(value, alive, dead); + element_wrapper e(value, alive, dead); auto it = this->container.insert(std::make_pair(key, e)); return MapFilterIterator>( it.first, diff --git a/libopenage/curve/map_filter_iterator.h b/libopenage/curve/map_filter_iterator.h index 5e71c0789f..e60f762c26 100644 --- a/libopenage/curve/map_filter_iterator.h +++ b/libopenage/curve/map_filter_iterator.h @@ -1,4 +1,4 @@ -// Copyright 2017-2023 the openage authors. See copying.md for legal info. +// Copyright 2017-2024 the openage authors. See copying.md for legal info. #pragma once @@ -35,8 +35,8 @@ class MapFilterIterator : public CurveIterator { using CurveIterator::operator=; virtual bool valid() const override { - return (this->get_base()->second.alive >= this->from - and this->get_base()->second.dead < this->to); + return (this->get_base()->second.alive() >= this->from + and this->get_base()->second.dead() < this->to); } /** diff --git a/libopenage/curve/queue.h b/libopenage/curve/queue.h index 9604a76308..210670ee7b 100644 --- a/libopenage/curve/queue.h +++ b/libopenage/curve/queue.h @@ -13,6 +13,7 @@ #include "curve/iterator.h" #include "curve/queue_filter_iterator.h" +#include "curve/element_wrapper.h" #include "event/evententity.h" #include "time/time.h" #include "util/fixed_point.h" @@ -32,39 +33,11 @@ namespace curve { */ template class Queue : public event::EventEntity { - struct queue_wrapper { - // Insertion time of the element - time::time_t _alive; - // Erase time of the element - // TODO: this has to be mutable because erase() will complain otherwise - mutable time::time_t _dead; - // Element value - T value; - - queue_wrapper(const time::time_t &time, const T &value) : - _alive{time}, - _dead{time::TIME_MAX}, - value{value} {} - - const time::time_t &alive() const { - return _alive; - } - - const time::time_t &dead() const { - return _dead; - } - - // TODO: this has to be const because erase() will complain otherwise - void set_dead(const time::time_t &time) const { - _dead = time; - } - }; - public: /** * The underlaying container type. */ - using container_t = typename std::vector; + using container_t = typename std::vector>; /** * The index type to access elements in the container @@ -412,7 +385,7 @@ QueueFilterIterator> Queue::insert(const time::time_t &time, // Get the iterator to the insertion point iterator insertion_point = std::next(this->container.begin(), at); - insertion_point = this->container.insert(insertion_point, queue_wrapper{time, e}); + insertion_point = this->container.insert(insertion_point, element_wrapper{time, e}); // TODO: Inserting before any dead elements shoud reset their death time // since by definition, they cannot be popped before the new element From 54b2191c6c97346943b54bab08d48f78b2a0f259 Mon Sep 17 00:00:00 2001 From: jere8184 Date: Mon, 16 Dec 2024 20:18:25 +0000 Subject: [PATCH 2/6] Update windows-server CI to run tests --- .github/workflows/windows-server-2019.yml | 1 + .github/workflows/windows-server-2022.yml | 1 + 2 files changed, 2 insertions(+) diff --git a/.github/workflows/windows-server-2019.yml b/.github/workflows/windows-server-2019.yml index df9d96284b..7cd34e49b5 100644 --- a/.github/workflows/windows-server-2019.yml +++ b/.github/workflows/windows-server-2019.yml @@ -76,6 +76,7 @@ jobs: $DLL_PATH = Join-Path package dll | Resolve-Path cd package python -m openage --add-dll-search-path $DLL_PATH --version + ./run.exe test -a shell: pwsh - name: Publish build artifacts uses: actions/upload-artifact@v4 diff --git a/.github/workflows/windows-server-2022.yml b/.github/workflows/windows-server-2022.yml index bbc73b78c2..327c23584a 100644 --- a/.github/workflows/windows-server-2022.yml +++ b/.github/workflows/windows-server-2022.yml @@ -76,6 +76,7 @@ jobs: $DLL_PATH = Join-Path package dll | Resolve-Path cd package python -m openage --add-dll-search-path $DLL_PATH --version + ./run.exe test -a shell: pwsh - name: Publish build artifacts uses: actions/upload-artifact@v4 From 727fd328ce6ae94e9c9a9da268cb052d5a711faf Mon Sep 17 00:00:00 2001 From: heinezen Date: Wed, 25 Dec 2024 14:41:39 +0100 Subject: [PATCH 3/6] curve: Add empty .cpp file for cmake. --- libopenage/curve/CMakeLists.txt | 1 + libopenage/curve/element_wrapper.cpp | 9 +++++++++ 2 files changed, 10 insertions(+) create mode 100644 libopenage/curve/element_wrapper.cpp diff --git a/libopenage/curve/CMakeLists.txt b/libopenage/curve/CMakeLists.txt index 1aa1efb55b..623a22a25a 100644 --- a/libopenage/curve/CMakeLists.txt +++ b/libopenage/curve/CMakeLists.txt @@ -3,6 +3,7 @@ add_sources(libopenage continuous.cpp discrete.cpp discrete_mod.cpp + element_wrapper.cpp interpolated.cpp iterator.cpp keyframe.cpp diff --git a/libopenage/curve/element_wrapper.cpp b/libopenage/curve/element_wrapper.cpp new file mode 100644 index 0000000000..5d2eaa08af --- /dev/null +++ b/libopenage/curve/element_wrapper.cpp @@ -0,0 +1,9 @@ +// Copyright 2024-2024 the openage authors. See copying.md for legal info. + +#include "element_wrapper.h" + +namespace openage::curve { + +// This file is intended to be empty + +} // namespace openage::curve From 71cdc50ce8cfbcafbfdf898d3189cb5dffc79f0a Mon Sep 17 00:00:00 2001 From: heinezen Date: Wed, 25 Dec 2024 14:43:43 +0100 Subject: [PATCH 4/6] curve: Fix comments of element wrapper. --- libopenage/curve/element_wrapper.h | 46 ++++++++++++++++++++++-------- 1 file changed, 34 insertions(+), 12 deletions(-) diff --git a/libopenage/curve/element_wrapper.h b/libopenage/curve/element_wrapper.h index 79756f0e5c..a0cfcd2109 100644 --- a/libopenage/curve/element_wrapper.h +++ b/libopenage/curve/element_wrapper.h @@ -7,45 +7,67 @@ namespace openage::curve { /** - * wrapper class for elements of a curve, store the insertion time and erase time of the element - * aswell the data of the element. + * Wrapper for elements in a curve container. + * + * Stores the lifetime of the element (insertion time and erasure time) alongside the value. */ template struct element_wrapper { - // Insertion time of the element + /// Time of insertion of the element into the container time::time_t _alive; - // Erase time of the element + /// Time of erasure of the element from the container time::time_t _dead; - // Element value + /// Element value T value; /** - * construct new element, set its start time and value - * its end time will be set to time::TIME_MAX + * Create a new element with insertion time \p time and a given value. + * + * Erasure time is set to time::TIME_MAX, i.e. the element is alive indefinitely. + * + * @param time Insertion time of the element. + * @param value Element value. */ element_wrapper(const time::time_t &time, const T &value) : _alive{time}, _dead{time::TIME_MAX}, value{value} {} - // construct new element, set its start time, end time and value + /** + * Create a new element with insertion time \p alive and erasure time \p dead and a given value. + * + * @param alive Insertion time of the element. + * @param dead Erasure time of the element. + * @param value Element value. + */ element_wrapper(const T &value, const time::time_t &alive, const time::time_t &dead) : _alive{alive}, _dead{dead}, value{value} {} - - // start time of this element + /** + * Get the insertion time of this element. + * + * @return Time when the element was inserted into the container. + */ const time::time_t &alive() const { return _alive; } - // end time of this element + /** + * Get the erasure time of this element. + * + * @return Time when the element was erased from the container. + */ const time::time_t &dead() const { return _dead; } - // set the end time of this element + /** + * Set the erasure time of this element. + * + * @param time Time when the element was erased from the container. + */ void set_dead(const time::time_t &time) { _dead = time; } From 41596023a1efc3e37ea682cb1ab8cbb4a3ce2843 Mon Sep 17 00:00:00 2001 From: heinezen Date: Wed, 25 Dec 2024 14:46:16 +0100 Subject: [PATCH 5/6] curve: Order arguments of both constructors the same way. --- libopenage/curve/element_wrapper.h | 2 +- libopenage/curve/map.h | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/libopenage/curve/element_wrapper.h b/libopenage/curve/element_wrapper.h index a0cfcd2109..498ee550cc 100644 --- a/libopenage/curve/element_wrapper.h +++ b/libopenage/curve/element_wrapper.h @@ -40,7 +40,7 @@ struct element_wrapper { * @param dead Erasure time of the element. * @param value Element value. */ - element_wrapper(const T &value, const time::time_t &alive, const time::time_t &dead) : + element_wrapper(const time::time_t &alive, const time::time_t &dead, const T &value) : _alive{alive}, _dead{dead}, value{value} {} diff --git a/libopenage/curve/map.h b/libopenage/curve/map.h index d996095d7c..5a5e0a4d9b 100644 --- a/libopenage/curve/map.h +++ b/libopenage/curve/map.h @@ -150,7 +150,7 @@ UnorderedMap::insert(const time::time_t &alive, const time::time_t &dead, const key_t &key, const val_t &value) { - element_wrapper e(value, alive, dead); + element_wrapper e{alive, dead, value}; auto it = this->container.insert(std::make_pair(key, e)); return MapFilterIterator>( it.first, From d6bf5b910ce0425d9cf9d9f1a991ebef1c937abc Mon Sep 17 00:00:00 2001 From: heinezen Date: Wed, 25 Dec 2024 14:56:03 +0100 Subject: [PATCH 6/6] curve: Make element wrapper member access private. Prevents unexpected manipulation. --- libopenage/curve/element_wrapper.h | 39 ++++++++++++++++++------ libopenage/curve/map_filter_iterator.h | 2 +- libopenage/curve/queue.h | 4 +-- libopenage/curve/queue_filter_iterator.h | 2 +- 4 files changed, 33 insertions(+), 14 deletions(-) diff --git a/libopenage/curve/element_wrapper.h b/libopenage/curve/element_wrapper.h index 498ee550cc..764b61d5cd 100644 --- a/libopenage/curve/element_wrapper.h +++ b/libopenage/curve/element_wrapper.h @@ -12,14 +12,8 @@ namespace openage::curve { * Stores the lifetime of the element (insertion time and erasure time) alongside the value. */ template -struct element_wrapper { - /// Time of insertion of the element into the container - time::time_t _alive; - /// Time of erasure of the element from the container - time::time_t _dead; - /// Element value - T value; - +class element_wrapper { +public: /** * Create a new element with insertion time \p time and a given value. * @@ -31,7 +25,7 @@ struct element_wrapper { element_wrapper(const time::time_t &time, const T &value) : _alive{time}, _dead{time::TIME_MAX}, - value{value} {} + _value{value} {} /** * Create a new element with insertion time \p alive and erasure time \p dead and a given value. @@ -43,7 +37,7 @@ struct element_wrapper { element_wrapper(const time::time_t &alive, const time::time_t &dead, const T &value) : _alive{alive}, _dead{dead}, - value{value} {} + _value{value} {} /** * Get the insertion time of this element. @@ -71,6 +65,31 @@ struct element_wrapper { void set_dead(const time::time_t &time) { _dead = time; } + + /** + * Get the value of this element. + * + * @return Value of the element. + */ + const T &value() const { + return _value; + } + +private: + /** + * Time of insertion of the element into the container + */ + time::time_t _alive; + + /** + * Time of erasure of the element from the container + */ + time::time_t _dead; + + /** + * Element value + */ + T _value; }; } // namespace openage::curve diff --git a/libopenage/curve/map_filter_iterator.h b/libopenage/curve/map_filter_iterator.h index e60f762c26..3aec2a899e 100644 --- a/libopenage/curve/map_filter_iterator.h +++ b/libopenage/curve/map_filter_iterator.h @@ -44,7 +44,7 @@ class MapFilterIterator : public CurveIterator { * Nicer way of accessing it beside operator *. */ val_t const &value() const override { - return this->get_base()->second.value; + return this->get_base()->second.value(); } /** diff --git a/libopenage/curve/queue.h b/libopenage/curve/queue.h index 210670ee7b..9314dd3a0e 100644 --- a/libopenage/curve/queue.h +++ b/libopenage/curve/queue.h @@ -277,7 +277,7 @@ const T &Queue::front(const time::time_t &time) const { << ", container size: " << this->container.size() << ")"); - return this->container.at(at).value; + return this->container.at(at).value(); } @@ -303,7 +303,7 @@ const T &Queue::pop_front(const time::time_t &time) { this->changes(time); - return this->container.at(at).value; + return this->container.at(at).value(); } diff --git a/libopenage/curve/queue_filter_iterator.h b/libopenage/curve/queue_filter_iterator.h index 248abb44ad..cf6bc5aa2c 100644 --- a/libopenage/curve/queue_filter_iterator.h +++ b/libopenage/curve/queue_filter_iterator.h @@ -40,7 +40,7 @@ class QueueFilterIterator : public CurveIteratorget_base(); - return a.value; + return a.value(); } };