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.

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 ASSERT macro, fix linux file paths in Taskfile and hopefully fix the windows release #1295

Merged
merged 10 commits into from
Apr 12, 2022
2 changes: 1 addition & 1 deletion .github/workflows/windows-workflow.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -85,7 +85,7 @@ jobs:
run: |
mkdir -p ./ci-artifacts/out
./.github/scripts/releases/extract_build_windows.sh ./ci-artifacts/out ./
7z a -tzip ./ci-artifacts/windows.zip ./ci-artfacts/out
7z a -tzip ./ci-artifacts/windows.zip ./ci-artifacts/out

- name: Upload Assets and Potential Publish Release
if: github.repository == 'open-goal/jak-project' && startsWith(github.ref, 'refs/tags/') && matrix.compiler == 'clang'
Expand Down
8 changes: 4 additions & 4 deletions Taskfile.yml
Original file line number Diff line number Diff line change
Expand Up @@ -13,27 +13,27 @@ tasks:
- '{{.DECOMP_BIN_RELEASE_DIR}}/decompiler "./decompiler/config/jak1_ntsc_black_label.jsonc" "./iso_data" "./decompiler_out" "decompile_code=false"'
boot-game:
preconditions:
- sh: test -f {{.DECOMP_BIN_RELEASE_DIR}}/gk{{.EXE_FILE_EXTENSION}}
- sh: test -f {{.GK_BIN_RELEASE_DIR}}/gk{{.EXE_FILE_EXTENSION}}
msg: "Couldn't locate runtime executable -- Have you compiled in release mode?"
cmds:
- "{{.GK_BIN_RELEASE_DIR}}/gk -boot -fakeiso -debug -v"
run-game:
preconditions:
- sh: test -f {{.DECOMP_BIN_RELEASE_DIR}}/gk{{.EXE_FILE_EXTENSION}}
- sh: test -f {{.GK_BIN_RELEASE_DIR}}/gk{{.EXE_FILE_EXTENSION}}
msg: "Couldn't locate runtime executable -- Have you compiled in release mode?"
cmds:
- "{{.GK_BIN_RELEASE_DIR}}/gk -fakeiso -debug -v"
run-game-quiet:
preconditions:
- sh: test -f {{.DECOMP_BIN_RELEASE_DIR}}/gk{{.EXE_FILE_EXTENSION}}
- sh: test -f {{.GK_BIN_RELEASE_DIR}}/gk{{.EXE_FILE_EXTENSION}}
msg: "Couldn't locate runtime executable -- Have you compiled in release mode?"
cmds:
- "{{.GK_BIN_RELEASE_DIR}}/gk -fakeiso"
repl:
env:
OPENGOAL_DECOMP_DIR: "jak1/"
preconditions:
- sh: test -f {{.DECOMP_BIN_RELEASE_DIR}}/goalc{{.EXE_FILE_EXTENSION}}
- sh: test -f {{.GOALC_BIN_RELEASE_DIR}}/goalc{{.EXE_FILE_EXTENSION}}
msg: "Couldn't locate compiler executable -- Have you compiled in release mode?"
cmds:
- "{{.GOALC_BIN_RELEASE_DIR}}/goalc"
Expand Down
3 changes: 1 addition & 2 deletions common/audio/audio_formats.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -289,8 +289,7 @@ void test_encode_adpcm(const std::vector<s16>& samples,
for (int i = 0; i < 5; i++) {
fmt::print(" [{}] {} {}\n", i, filter_errors[i], filter_shifts[i]);
}
fmt::print("prev: {} {}\n", prev_block_samples[0], prev_block_samples[1]);
ASSERT(false);
ASSERT_MSG(false, fmt::format("prev: {} {}", prev_block_samples[0], prev_block_samples[1]));
water111 marked this conversation as resolved.
Show resolved Hide resolved
}

prev_block_samples[0] = samples.at(block_idx * 28 + 27);
Expand Down
13 changes: 6 additions & 7 deletions common/custom_data/TFrag3Data.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -235,10 +235,9 @@ void Texture::serialize(Serializer& ser) {

void Level::serialize(Serializer& ser) {
ser.from_ptr(&version);
if (ser.is_loading() && version != TFRAG3_VERSION) {
fmt::print("version mismatch when loading tfrag3 data. Got {}, expected {}\n", version,
TFRAG3_VERSION);
ASSERT(false);
if (true) {
ASSERT_MSG(false, fmt::format("version mismatch when loading tfrag3 data. Got {}, expected {}",
version, TFRAG3_VERSION));
}

ser.from_str(&level_name);
Expand Down Expand Up @@ -285,9 +284,9 @@ void Level::serialize(Serializer& ser) {

ser.from_ptr(&version2);
if (ser.is_loading() && version2 != TFRAG3_VERSION) {
fmt::print("version mismatch when loading tfrag3 data (at end). Got {}, expected {}\n",
version2, TFRAG3_VERSION);
ASSERT(false);
ASSERT_MSG(false, fmt::format(
"version mismatch when loading tfrag3 data (at end). Got {}, expected {}",
version2, TFRAG3_VERSION));
}
}

Expand Down
4 changes: 1 addition & 3 deletions common/dma/dma.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -115,10 +115,8 @@ std::string VifCode::print() {
}

default:
fmt::print("Unhandled vif code {}\n", (int)kind);

result = "???";
ASSERT(false);
ASSERT_MSG(false, fmt::format("Unhandled vif code {}", (int)kind));
break;
}
// TODO: the rest of the VIF code.
Expand Down
3 changes: 1 addition & 2 deletions common/dma/dma.h
Original file line number Diff line number Diff line change
Expand Up @@ -91,8 +91,7 @@ inline void emulate_dma(const void* source_base, void* dest_base, u32 tadr, u32
// does this transfer anything in TTE???
return;
default:
printf("bad tag: %d\n", (int)tag.kind);
ASSERT(false);
ASSERT_MSG(false, fmt::format("bad tag: {}", (int)tag.kind));
}
}
}
Expand Down
15 changes: 13 additions & 2 deletions common/util/Assert.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,18 @@

#include "Assert.h"

void private_assert_failed(const char* expr, const char* file, int line, const char* function) {
fprintf(stderr, "%s:%d: Assertion failed: %s\nFunction: %s\n", file, line, expr, function);
void private_assert_failed(const char* expr,
const char* file,
int line,
const char* function,
const char* msg) {
if ((msg != NULL) && (msg[0] == '\0')) {
Copy link
Collaborator

Choose a reason for hiding this comment

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

I think this condition should be if( !msg || msg[0] == '\0') so we don't try to print a message if its null.

fprintf(stderr, "Assertion failed: '%s'\n\tSource: %s:%d\n\tFunction: %s\n", expr, file, line,
function);
} else {
fprintf(stderr, "Assertion failed: '%s'\n\tMessage: %s\n\tSource: %s:%d\n\tFunction: %s\n",
expr, msg, file, line, function);
}
fflush(stdout); // ensure any stdout logs are flushed before we terminate
Copy link
Collaborator

Choose a reason for hiding this comment

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

if you're paranoid, you could put fflush(stderr); as well

abort();
}
9 changes: 8 additions & 1 deletion common/util/Assert.h
Original file line number Diff line number Diff line change
Expand Up @@ -3,16 +3,23 @@
* Custom ASSERT macro
*/

#include "third-party/fmt/core.h"
Copy link
Collaborator

Choose a reason for hiding this comment

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

can we remove this? I'd rather not include libfmt in more places than we need because it's a huge header-only library that increases compile times. I think we can just put #include <string> instead.


#pragma once

[[noreturn]] void private_assert_failed(const char* expr,
const char* file,
int line,
const char* function);
const char* function,
const char* msg = "");

#ifdef _WIN32
#define __PRETTY_FUNCTION__ __FUNCSIG__
#endif

#define ASSERT(EX) \
(void)((EX) || (private_assert_failed(#EX, __FILE__, __LINE__, __PRETTY_FUNCTION__), 0))

#define ASSERT_MSG(EXPR, STR) \
(void)((EXPR) || \
(private_assert_failed(#EXPR, __FILE__, __LINE__, __PRETTY_FUNCTION__, STR.c_str()), 0))
Copy link
Collaborator

Choose a reason for hiding this comment

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

using .c_str() in the macro like this means it won't work for plain string constants. Maybe we could remove the .c_str() here and have a second private_assert_failed that takes const std::string& msg. This overload of private_assert_failed could then call the const char* msg version.

3 changes: 1 addition & 2 deletions common/util/FileUtil.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -425,8 +425,7 @@ void MakeISOName(char* dst, const char* src) {

void assert_file_exists(const char* path, const char* error_message) {
if (!std::filesystem::exists(path)) {
fprintf(stderr, "File %s was not found: %s\n", path, error_message);
ASSERT(false);
ASSERT_MSG(false, fmt::format("File {} was not found: {}", path, error_message));
}
}

Expand Down
120 changes: 90 additions & 30 deletions common/util/SmallVector.h
Original file line number Diff line number Diff line change
Expand Up @@ -61,8 +61,12 @@ class SmallVector {
typename std::aligned_storage<sizeof(T), alignof(T)>::type m_inline[inline_elt_count];

// get a T* at the beginning of our inline storage.
constexpr const T* inline_begin() const { return launder(reinterpret_cast<const T*>(m_inline)); }
constexpr T* inline_begin() { return launder(reinterpret_cast<T*>(m_inline)); }
constexpr const T* inline_begin() const {
return launder(reinterpret_cast<const T*>(m_inline));
}
constexpr T* inline_begin() {
return launder(reinterpret_cast<T*>(m_inline));
}

// regardless of our storage mode, these hold the beginning and end of the storage.
// by default, they are initialized to the inline storage.
Expand All @@ -86,7 +90,9 @@ class SmallVector {
/*!
* Free heap storage, without calling destructors of objects.
*/
void free_heap_storage(T* ptr) { delete[] launder(reinterpret_cast<uint8_t*>(ptr)); }
void free_heap_storage(T* ptr) {
delete[] launder(reinterpret_cast<uint8_t*>(ptr));
}

/*!
* Set the current storage to the inline memory.
Expand All @@ -98,7 +104,9 @@ class SmallVector {
m_storage_end = inline_begin() + inline_elt_count;
}

bool using_heap_storage() const { return m_storage_begin != inline_begin(); }
bool using_heap_storage() const {
return m_storage_begin != inline_begin();
}

/*!
* Allocate (if needed) storage to hold size objects.
Expand Down Expand Up @@ -200,7 +208,9 @@ class SmallVector {
/*!
* Constructor a SmallVector. By default the size is 0 and no elements are constructed.
*/
SmallVector() { initialize_storage_and_size(0); }
SmallVector() {
initialize_storage_and_size(0);
}

/*!
* Initialize vector with count elements that are default initialized
Expand Down Expand Up @@ -340,7 +350,9 @@ class SmallVector {
/*!
* See note on other assign.
*/
void assign(std::initializer_list<T> lst) { *this = SmallVector<T, inline_elt_count>(lst); }
void assign(std::initializer_list<T> lst) {
*this = SmallVector<T, inline_elt_count>(lst);
}

/*!
* Get the element at the index. Assert the index is valid.
Expand All @@ -361,59 +373,103 @@ class SmallVector {
/*!
* Get the element at the index. No range checking.
*/
T& operator[](std::size_t idx) { return m_storage_begin[idx]; }
T& operator[](std::size_t idx) {
return m_storage_begin[idx];
}

/*!
* Get the element at the index. No range checking.
*/
const T& operator[](std::size_t idx) const { return m_storage_begin[idx]; }
const T& operator[](std::size_t idx) const {
return m_storage_begin[idx];
}

/*!
* Get the first element. Not valid if size == 0
*/
T& front() { return m_storage_begin[0]; }
T& front() {
return m_storage_begin[0];
}

/*!
* Get the first element. Not valid if size == 0
*/
const T& front() const { return m_storage_begin[0]; }
const T& front() const {
return m_storage_begin[0];
}

/*!
* Get the last element. Not valid if size == 0
*/
T& back() { return m_storage_begin[m_size - 1]; }
T& back() {
return m_storage_begin[m_size - 1];
}

/*!
* Get the last element. Not valid if size == 0
*/
const T& back() const { return m_storage_begin[m_size - 1]; }
const T& back() const {
return m_storage_begin[m_size - 1];
}

/*!
* Get a pointer to data. This may become invalid after any resize, just like a std::vector.
*/
T* data() { return m_storage_begin; }
T* data() {
return m_storage_begin;
}

/*!
* Get a pointer to data. This may become invalid after any resize, just like a std::vector.
*/
const T* data() const { return m_storage_begin; }
const T* data() const {
return m_storage_begin;
}

iterator begin() { return m_storage_begin; }
const_iterator begin() const { return m_storage_begin; }
const_iterator cbegin() const { return m_storage_begin; }
iterator end() { return m_storage_begin + m_size; }
const_iterator end() const { return m_storage_begin + m_size; }
const_iterator cend() const { return m_storage_begin + m_size; }
iterator begin() {
return m_storage_begin;
}
const_iterator begin() const {
return m_storage_begin;
}
const_iterator cbegin() const {
return m_storage_begin;
}
iterator end() {
return m_storage_begin + m_size;
}
const_iterator end() const {
return m_storage_begin + m_size;
}
const_iterator cend() const {
return m_storage_begin + m_size;
}

reverse_iterator rbegin() { return m_storage_begin + m_size; }
const_reverse_iterator rbegin() const { return m_storage_begin + m_size; }
const_reverse_iterator crbegin() const { return m_storage_begin + m_size; }
reverse_iterator rend() { return m_storage_begin; }
const_reverse_iterator rend() const { return m_storage_begin; }
const_reverse_iterator crend() const { return m_storage_begin; }
reverse_iterator rbegin() {
return m_storage_begin + m_size;
}
const_reverse_iterator rbegin() const {
return m_storage_begin + m_size;
}
const_reverse_iterator crbegin() const {
return m_storage_begin + m_size;
}
reverse_iterator rend() {
return m_storage_begin;
}
const_reverse_iterator rend() const {
return m_storage_begin;
}
const_reverse_iterator crend() const {
return m_storage_begin;
}

bool empty() const { return m_size == 0; }
std::size_t size() const { return m_size; }
bool empty() const {
return m_size == 0;
}
std::size_t size() const {
return m_size;
}

/*!
* Make sure we have enough storage to hold desired_count without allocating.
Expand All @@ -439,7 +495,9 @@ class SmallVector {
/*!
* The number of elements we can hold without allocating more memory.
*/
std::size_t capacity() const { return m_storage_end - m_storage_begin; }
std::size_t capacity() const {
return m_storage_end - m_storage_begin;
}

/*!
* Shrink our heap allocation to the smallest possible size that can possibly hold the
Expand Down Expand Up @@ -544,7 +602,9 @@ class SmallVector {
m_size++;
}

void pop_back() { m_size--; }
void pop_back() {
m_size--;
}

void relocate_some_and_destroy_rest(T* dst,
T* src,
Expand Down
12 changes: 9 additions & 3 deletions common/util/Timer.h
Original file line number Diff line number Diff line change
Expand Up @@ -26,9 +26,13 @@ class Timer {
/*!
* Get milliseconds elapsed
*/
double getMs() const { return (double)getNs() / 1.e6; }
double getMs() const {
return (double)getNs() / 1.e6;
}

double getUs() const { return (double)getNs() / 1.e3; }
double getUs() const {
return (double)getNs() / 1.e3;
}

/*!
* Get nanoseconds elapsed
Expand All @@ -38,7 +42,9 @@ class Timer {
/*!
* Get seconds elapsed
*/
double getSeconds() const { return (double)getNs() / 1.e9; }
double getSeconds() const {
return (double)getNs() / 1.e9;
}

struct timespec _startTime = {};
};
Loading