-
Notifications
You must be signed in to change notification settings - Fork 94
Backport VSS to PC version #650
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
base: master
Are you sure you want to change the base?
Conversation
| int pitch; | ||
| SDL_LockTexture(sdlTexture, NULL, &pixels, &pitch); | ||
| blitRgba((uint32_t*)pixels, get_default_render_buffer(), get_2d_rgba_render_buffer(), get_2d_render_buffer()); | ||
| sys_frameQuant(pixels, xgrScreenSizeX, xgrScreenSizeY, 4); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Не понятно, может ли JS писать в буфер в этом случае. Для PC версии выглядит странным и избыточно.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
sys_frameQuant - оповещает js что фрейм изменился. В буфер js писать не может, но укзатель используется для чтения буфера.
Указатель может быть использован в функции getRgbaData что бы получить RGBA массив. Используется в андроид версии для отображения мелких экранов (меню, карты) в увеличенном виде.
| dictionary * iniparser_load(const char * ininame) | ||
| dictionary * iniparser_load(const char * _ininame) | ||
| { | ||
| const char* ininame = sys_fileOpenQuant(_ininame, /*XS::IN*/ 1); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Внутри делается duk_pop(ctx); что приводит к тому что итоговый указатель на строку может уже не существовать. Скорее всего надо копировать.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
вроде бы нет, используется duk_push_string, он копирует строку
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
confirm
| int XStream::open(const char* _name, unsigned f) | ||
| { | ||
|
|
||
| const char* name = sys_fileOpenQuant(_name, f); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
potential undefined behavior
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
может быть, лучше наверное std::string возвращать
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
папка /src/vss должна быть в /lib
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Кроме того .clang-format должен быть консистентным с глобальным. Или просто быть убран.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
у нас вроде как нет .clang-format
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
| #include <ctype.h> | ||
| #include "iniparser.h" | ||
|
|
||
| extern const char* sys_fileOpenQuant(const char* file, unsigned flags); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Надо избегать использовать позднего связывания и extern.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Структура проекта не позволяет сделать подругому. vss находится в основном проекте, соответсвенно нет возможности явно сослаться на него.
| id = xtInitApplication(); | ||
|
|
||
| if (!sys_readyQuant()) { | ||
| xtDoneApplication(); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Это штатный выход или нет? Не кажется что тут может быть штатный выход.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
такой же выход как и ниже, цикл игры пропщен. надо с asan запустить, мне кажется что все норм тут.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
убрать
|
|
||
| void xtClearMessageQueue(void) | ||
| { | ||
| sys_tickQuant(); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Почему оно тут а не основном цикле? xtClearMessageQueue вызывается так же во время разных анимаций в меню и эскейве.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
возможно основной цикл вызывается не достаточно часто, в андроиде используется для изменения перключения видео в магазине
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
перенести в основной цикл, пока не поймём
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Этот файл разве не должен быть в data? А тут может быть example?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
да
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
разве не должен быть этот файл в некой папке example?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
да
| .prop("rudder", rudder) | ||
| .prop("tractionIncrement", traction_increment) | ||
| .prop("tractionDecrement", traction_decrement) | ||
| .prop("tractionMax", 255) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Кажется тут мы должны некую переменную передавать либо иметь константу уже в самом vss.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
да
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
подставить константу
| "${PROJECT_SOURCE_DIR}/lib/xtool" | ||
| "${PROJECT_SOURCE_DIR}/lib/xgraph" | ||
| "${PROJECT_SOURCE_DIR}/lib/xsound" | ||
| "${PROJECT_SOURCE_DIR}/src/vss/duktape-2.7.0" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Этот движок должен подключаться как зависимость и не быть частью проекта.
| ${WINDOWS_RES} | ||
| actint/layout.h) | ||
|
|
||
| SET(vss_SRCS |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Надо отдельно сделать CMakeFile для vss и собирать отдельную .a библиотеку.
src/actint/actint.cpp
Outdated
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
не верный отступ пробелами тут
src/actint/actint.cpp
Outdated
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
лишний пробел
src/actint/actint.cpp
Outdated
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
зачем мы отправляем указатель в js и как мы с ним там работаем?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
можно удалить, никак не используется
src/iscreen/iscreen.cpp
Outdated
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Зачем нужуен этот квант? Тем более это просто redraw функция а не квант.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Используется для определния активного экрана. Используется в андроид версии для вывода Credits андроид версии перед основными кредитами
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
нужен для оверлея
src/iscreen/iscreen.cpp
Outdated
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Кажется не безопасно блокировать перерисовку из JS для элементов UI, какой был кейс?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Прячет не рабочие кнопки вроде, сетевой игры, fullscreen, etc...
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Поправка, это вроде не используется в андроид коде
src/iscreen/iscreen.cpp
Outdated
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
зачем это?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Прячет не рабочие кнопки вроде, сетевой игры, fullscreen, etc...
src/iscreen/iscreen.cpp
Outdated
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
похожий вопросс.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Прячет не рабочие кнопки вроде, сетевой игры, fullscreen, etc...
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
duktape не должен быть в репозитории.
src/vss/quant-names.h
Outdated
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
зачем constexpr тут?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
честно говоря не знаю. )
src/vss/sys-bridge.cpp
Outdated
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
зачем это тут?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Есть mod который выранвниает землю перед машиной.
src/vss/sys-bridge.cpp
Outdated
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
это зачем?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Есть mod который выранвниает землю перед машиной.
src/vss/sys-bridge.cpp
Outdated
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
аналогично.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
src/vss/sys-bridge.cpp
Outdated
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
разве в js мы нет уже готовой функции для этого?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
мы где то разве используем base64?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Так мы и создаем эту функцию в js) Вроде не используется, но думаю 100% понадобится
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
пока убираем
src/vss/sys-quant.cpp
Outdated
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
потенциальная проблемма
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
yes

Probably github don't ignore line spacing by default, please use this option to see actual diff: