Skip to content

Add file check#84

Merged
Gr-i-niy merged 12 commits into
mainfrom
add-file-check
Jun 2, 2025
Merged

Add file check#84
Gr-i-niy merged 12 commits into
mainfrom
add-file-check

Conversation

@Fra1sse

@Fra1sse Fra1sse commented Apr 5, 2025

Copy link
Copy Markdown
Collaborator

Добавлена проверка на существование загружаемых файлов в vk_loader.cpp и vk_pipelines.cpp

@anunknowperson anunknowperson requested a review from Copilot April 5, 2025 22:11

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Copilot reviewed 3 out of 3 changed files in this pull request and generated no comments.

Comments suppressed due to low confidence (1)

src/graphics/vulkan/vk_loader.cpp:210

  • [nitpick] Consider using spdlog for logging instead of std::cout to maintain consistency with the rest of the file's logging.
std::cout << "Loading GLTF " << filePath << '\n';

Comment thread src/graphics/vulkan/vk_pipelines.cpp Outdated
@alistkova

Copy link
Copy Markdown
Contributor

ну норм

homka122

This comment was marked as duplicate.

@homka122 homka122 left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

хайп

Comment thread src/core/ModelImpl.cpp

@NikitosKey NikitosKey left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Под пиво норм.

@anunknowperson anunknowperson left a comment

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

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

Заменить прямой вызов spdlog на макросы которые едины для всего проекта

#include "core/Logging.h"

LOGI(message)

LOGE("{}", message)

@anunknowperson anunknowperson left a comment

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

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

Посмотри есть ли где-то ещё в проекте вызов cout и там замени на LOGI

@NikitosKey

Copy link
Copy Markdown

Заменить прямой вызов spdlog на макросы которые едины для всего проекта

#include "core/Logging.h"

LOGI(message)

LOGE("{}", message)

Фу, макросы в плюсах

@georgiy-belyanin georgiy-belyanin left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Thank you for working on it and forming such a nice patch set! LGTM

Comment thread src/core/ModelImpl.cpp
@anunknowperson

anunknowperson commented Apr 7, 2025

Copy link
Copy Markdown
Owner

Заменить прямой вызов spdlog на макросы которые едины для всего проекта
#include "core/Logging.h"
LOGI(message)
LOGE("{}", message)

Фу, макросы в плюсах

вроде мы обсуждали макросы и нам разрешили их оставить? не помню. В любом случае, если и менять то это для другой таски. Сейчас во всем проекте юзают макросы.

@Tydik42 Tydik42 self-requested a review April 7, 2025 18:54
@NikitosKey

Copy link
Copy Markdown

Заменить прямой вызов spdlog на макросы которые едины для всего проекта
#include "core/Logging.h"
LOGI(message)
LOGE("{}", message)

Фу, макросы в плюсах

вроде мы обсуждали макросы и нам разрешили их оставить? не помню. В любом случае, если и менять то это для другой таски. Сейчас во всем проекте юзают макросы.

Хз, вроде в плюсах неймспейсы для этого всего придуманы. Но в целом, если везде одинаково, то пох. Лучше так, чем везде как попало.

@Fra1sse Fra1sse requested a review from anunknowperson April 7, 2025 19:42

@maybenotilya maybenotilya left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Огромное уважение автору данного Pull Request'а за неоценимый вклад в развитие рендерлибы

@Ycyken Ycyken left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

awesome bro you're amazing

@georgiy-belyanin

georgiy-belyanin commented May 2, 2025

Copy link
Copy Markdown

Hey! Any updates on this one?

1 similar comment
@maybenotilya

Copy link
Copy Markdown

Hey! Any updates on this one?

@anunknowperson anunknowperson left a comment

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

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

lgtm

@Gr-i-niy Gr-i-niy merged commit 61f1687 into main Jun 2, 2025
4 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.