address comments

libopenage/curve/array.h

@@ -16,10 +16,10 @@ template <typename T, size_t Size>
 class Array : event::EventEntity {
 public:
 	Array(const std::shared_ptr<event::EventLoop> &loop,
-	          size_t id,
-	          const std::string &idstr = "",
-	          const EventEntity::single_change_notifier &notifier = nullptr) :
-		EventEntity(loop, notifier), _id{id}, _idstr{idstr}, loop{loop}{}
+	      size_t id,
+	      const std::string &idstr = "",
+	      const EventEntity::single_change_notifier &notifier = nullptr) :
+		EventEntity(loop, notifier), _id{id}, _idstr{idstr}, loop{loop} {}
 
 	Array(const Array &) = delete;
 
@@ -28,16 +28,16 @@ class Array : event::EventEntity {
 	 * Get the last element with elem->time <= time from
 	 * the keyfram container at a given index
 	 */
-	T get(const time::time_t &t, const size_t index) const;
+	T at(const time::time_t &t, const size_t index) const;
 
 
 	/**
 	 * Get an array of the last elements with elem->time <= time from
 	 * all keyfram containers contained within this array curve
 	 */
-	std::array<T, Size> get_all(const time::time_t &t) const;
-
+	std::array<T, Size> get(const time::time_t &t) const;
 
+	// Get the amount of KeyframeContainers in array curve
 	consteval size_t size() const;
 
 	/**
@@ -107,92 +107,107 @@ class Array : event::EventEntity {
 	}
 
 
-	KeyframeContainer<T>& operator[] (size_t index)
-	{
-		return this->container[index];
-	}
-
-	KeyframeContainer<T> operator[] (size_t index) const
-	{
-		return this->container[index];
+	// get a copy to the KeyframeContainer at index
+	KeyframeContainer<T> operator[](size_t index) const {
+		return this->container.at(index);
 	}
 
+	// Array::Iterator is used to iterate over KeyframeContainers contained in a curve at a given time.
 	class Iterator {
 	public:
 		Iterator(Array<T, Size> *curve, const time::time_t &time = time::TIME_MAX, size_t offset = 0) :
 			curve(curve), time(time), offset(offset) {};
 
+		// returns a copy of the keyframe at the current offset and time
 		const T operator*() {
-			return curve->frame(this->time, this->offset).second;
+			return this->curve->frame(this->time, this->offset).second;
 		}
 
+		// increments the Iterator to point at the next KeyframeContainer
 		void operator++() {
 			this->offset++;
 		}
 
+		// Compare two Iterators by their offset
 		bool operator!=(const Array<T, Size>::Iterator &rhs) const {
 			return this->offset != rhs.offset;
 		}
 
 
 	private:
+		// used to index the Curve::Array pointed to by this iterator
 		size_t offset;
+
+		// curve::Array that this iterator is iterating over
 		Array<T, Size> *curve;
+
+		// time at which this iterator is iterating over
 		time::time_t time;
 	};
 
 
-	Iterator begin(const time::time_t &time = time::TIME_MAX);
+	// iterator pointing to a keyframe of the first KeyframeContainer in the curve at a given time
+	Iterator begin(const time::time_t &time = time::TIME_MIN);
 
-	Iterator end(const time::time_t &time = time::TIME_MAX);
+	// iterator pointing after the last KeyframeContainer in the curve at a given time
+	Iterator end(const time::time_t &time = time::TIME_MIN);
 
 
 private:
 	std::array<KeyframeContainer<T>, Size> container;
 
-	//hint for KeyframeContainer operations
+	/**
+	 * hints for KeyframeContainer operations, mutable as hints
+	 * are updated by const read functions.
+	 * This is used to speed up the search for next keyframe to be accessed
+	 */
 	mutable std::array<size_t, Size> last_element = {};
 
 	/**
-	* Identifier for the container
-	*/
+	 * Identifier for the container
+	 */
 	const size_t _id;
 
 	/**
-	* Human-readable identifier for the container
-	*/
+	 * Human-readable identifier for the container
+	 */
 	const std::string _idstr;
 
 	/**
-	* The eventloop this curve was registered to
-	*/
+	 * The eventloop this curve was registered to
+	 */
 	const std::shared_ptr<event::EventLoop> loop;
 };
 
 
 template <typename T, size_t Size>
 std::pair<time::time_t, T> Array<T, Size>::frame(const time::time_t &t, const size_t index) const {
-	size_t frmae_index = container[index].last(t, this->last_element[index]);
-	this->last_element[index] = frmae_index;
-	return container[index].get(frmae_index).make_pair();
+	size_t &hint = this->last_element[index];
+	size_t frame_index = this->container.at(index).last(t, hint);
+	hint = frame_index;
+	return this->container.at(index).get(frame_index).make_pair();
 }
 
 template <typename T, size_t Size>
 std::pair<time::time_t, T> Array<T, Size>::next_frame(const time::time_t &t, const size_t index) const {
-	size_t frmae_index = container[index].last(t, this->last_element[index]);
-	this->last_element[index] = frmae_index;
-	return container[index].get(frmae_index + 1).make_pair();
+	size_t &hint = this->last_element[index];
+	size_t frame_index = this->container.at(index).last(t, hint);
+	hint = frame_index;
+	return this->container.at(index).get(frame_index + 1).make_pair();
 }
 
 template <typename T, size_t Size>
-T Array<T, Size>::get(const time::time_t &t, const size_t index) const {
-	return this->frame(t, index).second;
+T Array<T, Size>::at(const time::time_t &t, const size_t index) const {
+	size_t &hint = this->last_element[index];
+	size_t frame_index = this->container.at(index).last(t, hint);
+	hint = frame_index;
+	return this->container.at(index).get(frame_index).val();
 }
 
 template <typename T, size_t Size>
-std::array<T, Size> Array<T, Size>::get_all(const time::time_t &t) const {
+std::array<T, Size> Array<T, Size>::get(const time::time_t &t) const {
 	return [&]<auto... I>(std::index_sequence<I...>) {
-		return std::array<T, Size>{this->get(t, I)...};
+		return std::array<T, Size>{this->at(t, I)...};
 	}(std::make_index_sequence<Size>{});
 }
 
@@ -204,22 +219,21 @@ consteval size_t Array<T, Size>::size() const {
 
 template <typename T, size_t Size>
 void Array<T, Size>::set_insert(const time::time_t &t, const size_t index, T value) {
-	this->last_element[index] = this->container[index].insert_after(Keyframe(t, value), this->last_element[index]);
+	this->last_element[index] = this->container.at(index).insert_after(Keyframe{t, value}, this->last_element[index]);
 }
 
 
 template <typename T, size_t Size>
 void Array<T, Size>::set_last(const time::time_t &t, const size_t index, T value) {
-
-	size_t frame_index = this->container[index].insert_after(Keyframe(t, value), this->last_element[index]);
+	size_t frame_index = this->container.at(index).insert_after(Keyframe{t, value}, this->last_element[index]);
 	this->last_element[index] = frame_index;
-	this->container[index].erase_after(frame_index);
+	this->container.at(index).erase_after(frame_index);
 }
 
 
 template <typename T, size_t Size>
 void Array<T, Size>::set_replace(const time::time_t &t, const size_t index, T value) {
-	this->container[index].insert_overwrite(Keyframe(t, value), this->last_element[index]);
+	this->container.at(index).insert_overwrite(Keyframe{t, value}, this->last_element[index]);
 }
 
 template <typename T, size_t Size>

libopenage/curve/base_curve.h

@@ -245,7 +245,7 @@ template <typename T>
 std::pair<time::time_t, const T> BaseCurve<T>::frame(const time::time_t &time) const {
 	auto e = this->container.last(time, this->container.size());
 	auto elem = this->container.get(e);
-	return std::make_pair(elem.time(), elem.val());
+	return elem.make_pair();
 }
 
 
@@ -254,7 +254,7 @@ std::pair<time::time_t, const T> BaseCurve<T>::next_frame(const time::time_t &ti
 	auto e = this->container.last(time, this->container.size());
 	e++;
 	auto elem = this->container.get(e);
-	return std::make_pair(elem.time(), elem.val());
+	return elem.make_pair();
 }
 
 template <typename T>

libopenage/curve/tests/container.cpp

@@ -9,6 +9,7 @@
 #include <unordered_map>
 #include <unordered_set>
 
+#include "curve/array.h"
 #include "curve/iterator.h"
 #include "curve/map.h"
 #include "curve/map_filter_iterator.h"
@@ -56,7 +57,7 @@ void test_map() {
 
 	// Basic tests test lookup in the middle of the range.
 	{
-		auto t = map.at(2, 0); //At timestamp 2 element 0
+		auto t = map.at(2, 0); // At timestamp 2 element 0
 		TESTEQUALS(t.has_value(), true);
 		TESTEQUALS(t.value().value(), 0);
 		t = map.at(20, 5);
@@ -242,11 +243,92 @@ void test_queue() {
 	TESTEQUALS(q.empty(100001), false);
 }
 
+void test_array() {
+	auto f = std::make_shared<event::EventLoop>();
+
+	Array<int, 4> a(f, 0);
+	a.set_insert(1, 0, 0);
+	a.set_insert(1, 1, 1);
+	a.set_insert(1, 2, 2);
+	a.set_insert(1, 3, 3);
+	// a = [[0:0, 1:0],[0:0, 1:1],[0:0, 1:2],[0:0, 1:3]]
+
+	auto res = a.get(1);
+	TESTEQUALS(res.at(0), 0);
+	TESTEQUALS(res.at(1), 1);
+	TESTEQUALS(res.at(2), 2);
+	TESTEQUALS(res.at(3), 3);
+
+	Array<int, 4> other(f, 0);
+	other.set_last(0, 0, 999);
+	other.set_last(0, 1, 999);
+	other.set_last(0, 2, 999);
+	other.set_last(0, 3, 999);
+
+	other.set_insert(1, 0, 4);
+	other.set_insert(1, 1, 5);
+	other.set_insert(1, 2, 6);
+	other.set_insert(1, 3, 7);
+	// other = [[0:999, 1:4],[0:999, 1:5],[0:999, 1:6],[0:999, 1:7]]
+
+
+	a.sync(other, 1);
+	// a = [[0:0, 1:4],[0:0, 1:5],[0:0, 1:6],[0:0, 1:7]]
+
+	res = a.get(0);
+	TESTEQUALS(res.at(0), 0);
+	TESTEQUALS(res.at(1), 0);
+	TESTEQUALS(res.at(2), 0);
+	TESTEQUALS(res.at(3), 0);
+	res = a.get(1);
+	TESTEQUALS(res.at(0), 4);
+	TESTEQUALS(res.at(1), 5);
+	TESTEQUALS(res.at(2), 6);
+	TESTEQUALS(res.at(3), 7);
+
+	// Additional tests
+	a.set_insert(2, 0, 15);
+	a.set_insert(2, 0, 20);
+	a.set_replace(2, 0, 25);
+	TESTEQUALS(a.at(2, 0), 25);
+	// a = [[0:0, 1:4, 2:25],[0:0, 1:5],[0:0, 1:6],[0:0, 1:7]]
+
+	a.set_insert(3, 0, 30);
+	a.set_insert(4, 0, 40);
+	a.set_last(3, 0, 35);
+	TESTEQUALS(a.at(4, 0), 35);
+	// a = [[0:0, 1:4, 2:25, 3:35],[0:0, 1:5],[0:0, 1:6],[0:0, 1:7]]
+
+	auto frame = a.frame(1, 2);
+	TESTEQUALS(frame.second, 6);
+	TESTEQUALS(frame.first, 1);
+
+	a.set_insert(5, 3, 40);
+	auto next_frame = a.next_frame(1, 3);
+	TESTEQUALS(next_frame.second, 40);
+	TESTEQUALS(next_frame.first, 5);
+
+	// Test operator[]
+	TESTEQUALS(a[0].get(a[0].last(2)).val(), 25);
+	TESTEQUALS(a[1].get(a[1].last(2)).val(), 5);
+
+	// Test begin and end
+	auto it = a.begin(1);
+	TESTEQUALS(*it, 4);
+	++it;
+	TESTEQUALS(*it, 5);
+	++it;
+	TESTEQUALS(*it, 6);
+	++it;
+	TESTEQUALS(*it, 7);
+}
+
 
 void container() {
 	test_map();
 	test_list();
 	test_queue();
+	test_array();
 }