Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Improve Container Types #180

Open
wants to merge 8 commits into
base: master
Choose a base branch
from
Open

Improve Container Types #180

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
8 commits
Select commit Hold shift + click to select a range
4756017
Adjust file paths
Mozz3d Feb 7, 2025
880f4bd
Improve Container Types
Mozz3d Feb 7, 2025
b6f4513
Fix include path
Mozz3d Feb 7, 2025
937f56d
Adjustments
Mozz3d Feb 7, 2025
6080dc9
Several improvements & adjustments
Mozz3d Feb 11, 2025
00a2473
Fix mistake & ArrayIterator crediting
Mozz3d Feb 11, 2025
3ae9d42
Fix errors
Mozz3d Feb 11, 2025
9a904b4
Fix compiler warning
Mozz3d Feb 11, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
The table of contents is too big for display.
Diff view
Diff view
  •  
  •  
  •  
550 changes: 550 additions & 0 deletions include/RED4ext/Containers/DynArray.hpp
View file
Open in desktop
wopss marked this conversation as resolved.
Show resolved Hide resolved

Large diffs are not rendered by default.

204 changes: 204 additions & 0 deletions include/RED4ext/Containers/Span.hpp
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,204 @@
#pragma once

#include <algorithm>
#include <cassert>
#include <cstdint>
#include <stdexcept>

#include <RED4ext/Common.hpp>
#include <RED4ext/Detail/Containers/ArrayIterator.hpp>

namespace RED4ext
{
template<typename T>
struct Span
{
using ValueType = T;
using Reference = ValueType&;
using ConstReference = const ValueType;
using Pointer = ValueType*;
using ConstPointer = const ValueType*;

using SizeType = std::uint32_t;
using DifferenceType = std::ptrdiff_t;

using Iterator = Detail::ArrayIterator<ValueType, Span>;
using ConstIterator = Detail::ArrayIterator<const ValueType, Span>;
using ReverseIterator = std::reverse_iterator<Iterator>;
using ConstReverseIterator = std::reverse_iterator<ConstIterator>;

Span()
: beginPtr(nullptr)
, endPtr(nullptr)
{
}

Span(Pointer aBegin, Pointer aEnd)
: beginPtr(aBegin)
, endPtr(aEnd)
{
}

Span(Pointer aBegin, SizeType aCount)
: beginPtr(aBegin)
, endPtr(aBegin + aCount)
{
}

constexpr Reference operator[](SizeType aPos)
{
assert(aPos < Size());
return Data()[aPos];
}

constexpr ConstReference operator[](SizeType aPos) const
{
assert(aPos < Size());
return Data()[aPos];
}

[[nodiscard]] constexpr Reference At(SizeType aPos)
{
if (aPos >= Size())
throw std::out_of_range("Span::At out of range");

return Data()[aPos];
}

[[nodiscard]] constexpr ConstReference At(SizeType aPos) const
{
if (aPos >= Size())
throw std::out_of_range("Span::At out of range");

return Data()[aPos];
}

[[nodiscard]] constexpr Iterator Find(ConstReference aValue) noexcept
{
return Iterator(std::find(begin(), end(), aValue));
}

[[nodiscard]] constexpr ConstIterator Find(ConstReference aValue) const noexcept
{
return ConstIterator(std::find(cbegin(), cend(), aValue));
}

[[nodiscard]] constexpr bool Contains(ConstReference aValue) const noexcept
{
return Find(aValue) != cend();
}

#pragma region STL
[[nodiscard]] constexpr Reference Front()
{
assert(!Empty());
return Data()[0];
}

[[nodiscard]] constexpr ConstReference Front() const
{
assert(!Empty());
return Data()[0];
}

[[nodiscard]] constexpr Reference Back()
{
assert(!Empty());
return Data()[Size() - 1];
}

[[nodiscard]] constexpr ConstReference Back() const
{
assert(!Empty());
return Data()[Size() - 1];
}
#pragma region Iterator
[[nodiscard]] constexpr Iterator begin() noexcept
{
return beginPtr;
}

[[nodiscard]] constexpr ConstIterator begin() const noexcept
{
return beginPtr;
}

[[nodiscard]] constexpr ConstIterator cbegin() const noexcept
{
return begin();
}

[[nodiscard]] constexpr Iterator end() noexcept
{
return endPtr;
}

[[nodiscard]] constexpr ConstIterator end() const noexcept
{
return endPtr;
}

[[nodiscard]] constexpr ConstIterator cend() const noexcept
{
return end();
}
#pragma endregion
#pragma region Reverse Iterator
[[nodiscard]] constexpr ReverseIterator rbegin() noexcept
{
return ReverseIterator(begin());
}

[[nodiscard]] constexpr ConstReverseIterator rbegin() const noexcept
{
return ConstReverseIterator(begin());
}

[[nodiscard]] constexpr ConstReverseIterator crbegin() const
{
return rbegin();
}

[[nodiscard]] constexpr ReverseIterator rend() noexcept
{
return ReverseIterator(end());
}

[[nodiscard]] constexpr ConstReverseIterator rend() const noexcept
{
return ConstReverseIterator(end());
}

[[nodiscard]] constexpr ConstReverseIterator crend() const noexcept
{
return rend();
}
#pragma endregion
[[nodiscard]] constexpr bool Empty() const
{
return !Data();
}

[[nodiscard]] constexpr Pointer Data() noexcept
{
return beginPtr;
}

[[nodiscard]] constexpr ConstPointer Data() const noexcept
{
return beginPtr;
}

[[nodiscard]] constexpr DifferenceType Size() const
{
return endPtr - beginPtr;
}
#pragma endregion

T* beginPtr; // 00
T* endPtr; // 08
};
RED4EXT_ASSERT_SIZE(Span<int32_t>, 0x10);
RED4EXT_ASSERT_OFFSET(Span<int32_t>, beginPtr, 0x0);
RED4EXT_ASSERT_OFFSET(Span<int32_t>, endPtr, 0x8);
} // namespace RED4ext
156 changes: 156 additions & 0 deletions include/RED4ext/Containers/StaticArray.hpp
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,156 @@
#pragma once

#include <algorithm>
#include <array>
#include <cassert>
#include <cstdint>

#include <RED4ext/Common.hpp>
#include <RED4ext/Detail/Containers/ArrayIterator.hpp>

namespace RED4ext
{
template<typename T, uint32_t MAX_LEN>
struct StaticArray
{
using ValueType = T;
using Reference = ValueType&;
using ConstReference = const ValueType&;
using Pointer = ValueType*;
using ConstPointer = const ValueType*;

using SizeType = std::uint32_t;
using DifferenceType = std::ptrdiff_t;

using Iterator = Detail::ArrayIterator<ValueType, StaticArray>;
using ConstIterator = Detail::ArrayIterator<const ValueType, StaticArray>;
using ReverseIterator = std::reverse_iterator<Iterator>;
using ConstReverseIterator = std::reverse_iterator<ConstIterator>;

constexpr Reference operator[](SizeType aPos)
{
assert(aPos < Size());
return Data()[aPos];
}

constexpr ConstReference operator[](SizeType aPos) const
{
assert(aPos < Size());
return Data()[aPos];
}

[[nodiscard]] constexpr Reference At(SizeType aPos)
{
if (aPos >= Size())
throw std::out_of_range("StaticArray::At out of range");

return Data()[aPos];
}

[[nodiscard]] constexpr ConstReference At(SizeType aPos) const
{
if (aPos >= Size())
throw std::out_of_range("StaticArray::At out of range");

return Data()[aPos];
}

#pragma region STL
#pragma region Iterator
constexpr Iterator begin() noexcept
{
return entries;
}

constexpr ConstIterator begin() const noexcept
{
return entries;
}

constexpr Iterator end() noexcept
{
return entries + size;
}

constexpr ConstIterator end() const noexcept
{
return entries + size;
}
#pragma endregion
#pragma region Reverse Iterator
[[nodiscard]] constexpr ReverseIterator rbegin() noexcept
{
return ReverseIterator(begin());
}

[[nodiscard]] constexpr ConstReverseIterator rbegin() const noexcept
{
return ConstReverseIterator(begin());
}

[[nodiscard]] constexpr ReverseIterator rend() noexcept
{
return ReverseIterator(end());
}

[[nodiscard]] constexpr ConstReverseIterator rend() const noexcept
{
return ConstReverseIterator(end());
}
#pragma endregion
[[nodiscard]] constexpr Reference Front()
{
assert(!Empty());
return Data()[0];
}

[[nodiscard]] constexpr ConstReference Front() const
{
assert(!Empty());
return Data()[0];
}

[[nodiscard]] constexpr Reference Back()
{
assert(!Empty());
return Data()[Size() - 1];
}

[[nodiscard]] constexpr ConstReference Back() const
{
assert(!Empty());
return Data()[Size() - 1];
}
Comment on lines +101 to +123
Copy link
Owner

Choose a reason for hiding this comment

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

Imo, things like begin, front, etc. should only be STL style to not have duplicated. Wdyt?

Same for DynArray, Span.

Copy link
Collaborator

Choose a reason for hiding this comment

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

Yeah definitely + from me on this.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Could you elaborate on what you mean a little? Im not sure if you're referring to just casing or something else aswell

Copy link
Collaborator

@psiberx psiberx Feb 11, 2025 •
edited
Loading

Choose a reason for hiding this comment

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

I think it should be either STL or our style (STL conformant or not), but not both, with only exceptions for compliance with named requirements, range-based for loop, etc. If we want to keep our naming, then it should be Size(), Begin(), Front(), etc. Things like begin() and end() should be in a separate "compliance" region, not mixed in with the rest. What's the purpose of lowercase front() btw?

Copy link
Owner

Choose a reason for hiding this comment

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

I don't think lowercase front has any purpose, but I made a typo, I wanted to say being, end, etc. Basically, everything that is needed for range-based loop.

For these requirements, I would say to just use the lowercase (STL variant) naming, instead of having both.

Copy link
Collaborator

Choose a reason for hiding this comment

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

I'd keep Begin and End for consistent naming (you still need to use them explicitly) and not consider begin and end as part of our interface.

Copy link
Collaborator

@WSSDude WSSDude Feb 12, 2025 •
edited
Loading

Choose a reason for hiding this comment

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

I dont know what you mean by consistent naming now - consistent with what? Other functions? Other collections not touched by this (if there are any)?

I would rather not have 2 functions doing the same thing personally, even if it would cause some function ToBeLikeThis and some like_this following std.

Mainly when they should be compatible with it, 2 of anything unnecessarily is a waste in my eyes.

Copy link
Collaborator

@psiberx psiberx Feb 12, 2025 •
edited
Loading

Choose a reason for hiding this comment

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

Consistent across the class at least, ideally across the project. begin and end should not be considered functions of a class, but unfortunately C++ doesn't have a way to handle it properly. For example, C# supports explicit interface implementation that would hide begin and end from class.
Begin and begin have different purposes, and that's what matters, how they're implemented is irrelevant. begin exists to support range-based for loop, for everything else there's Begin.


[[nodiscard]] constexpr bool Empty() const noexcept
{
return Size() == 0;
}

constexpr Pointer Data() noexcept
{
return entries;
}

constexpr ConstPointer Data() const noexcept
{
return entries;
}

constexpr SizeType MaxSize() const noexcept
{
return MAX_LEN;
}

constexpr SizeType Size() const noexcept
{
return size;
}
#pragma endregion

T entries[MAX_LEN]; // 00
uint32_t size;
};
static_assert(sizeof(StaticArray<std::array<uint8_t, 5>, 32>) ==
164); // StaticArray<GpuWrapApiVertexPackingPackingElement, 32>
} // namespace RED4ext
Loading