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

Add zip file support #41

Merged
merged 18 commits into from
Jun 2, 2019
Merged

Add zip file support #41

merged 18 commits into from
Jun 2, 2019

Conversation

Mr0maks
Copy link
Contributor

@Mr0maks Mr0maks commented May 25, 2019

No description provided.

@a1batross
Copy link
Member

Зачем?

@Mr0maks
Copy link
Contributor Author

Mr0maks commented May 25, 2019

Зачем?

А чего такого плохово в зип файлах ?

@a1batross
Copy link
Member

Ничего плохого. Просто для чего это использовать на уровне движка?

В целом мне код понравился, только кодстайл поправь. Если кратко: табы вместо пробелов, пробел перед скобками не надо ставить, надо только внутри(кроме когда они повторяются), фигурные скобки на новой строке.

Проверку на zlib в билдсистему бы завезти.
Мне кажется, не имеет смысла делать отключаемое сжатие, ибо zlib одна из тех либ, которая есть везде.

@nekonomicon
Copy link
Member

nekonomicon commented May 25, 2019

А чего сразу не zlib/gzip раз целую zlib притащил?
Но для этих целей есть более легкая miniz
Если же есть необходимость только в поддержке .pk3, можно обойтись чем-то проще.
Но вот намеков на расширение .pk3 в коде я не увидел, хотя это тот же zip.

@Mr0maks
Copy link
Contributor Author

Mr0maks commented May 25, 2019

Но вот намеков на расширение .pk3 в коде я не увидел, хотя это тот же zip.

Добавил

@Mr0maks
Copy link
Contributor Author

Mr0maks commented May 26, 2019

В целом мне код понравился, только кодстайл поправь.

Подправил в последнем коммите

@a1batross
Copy link
Member

Я видел.

Если не заметил, я повторю ещё раз: XASH_ZLIB не нужен. Можно вполне требовать zlib зависимостью или вложить его в сорцы, ибо лицензия позволяет. Или собрать свой miniz, удалив из него поддержку Zip(она теперь у нас и так своя).

Почему я за добавление zlib/miniz? В будущем это пригодится для поддержки PNG.

@Mr0maks
Copy link
Contributor Author

Mr0maks commented May 26, 2019

Если не заметил, я повторю ещё раз: XASH_ZLIB не нужен.

Убрал

engine/common/common.h Outdated Show resolved Hide resolved
@a1batross
Copy link
Member

Ну, вроде теперь норм.

@Mr0maks теперь пиши проверку на существование либы и хедера в engine/wscript и линковку. Тут помогу, в принципе:

def configure(conf):
	...
	conf.check_cc(header_name='zlib.h', uselib_store='ZLIB')
	conf.check_cc(lib='z', uselib_store='ZLIB')
	...

def build(bld):
	...
	libs = [ ..., 'ZLIB' ]
	...

Замержу как буду свободен. Завтра может. Может послезавтра.

engine/wscript Outdated Show resolved Hide resolved
@Mr0maks
Copy link
Contributor Author

Mr0maks commented May 30, 2019

Вроде всё уже готово

@a1batross a1batross merged commit 73cf465 into FWGS:master Jun 2, 2019
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.

4 participants