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 all.h and use in place of diablo.h #611

Closed
wants to merge 1 commit into from
Closed

add all.h and use in place of diablo.h #611

wants to merge 1 commit into from

Conversation

mewmew
Copy link
Contributor

@mewmew mewmew commented Feb 14, 2020

Now diablo.h is treated in the same way as all other header files of
Source, as it only contains the declarations of global variables and
functions of diablo.cpp.

Besides consistency, this also enables mods to include diablo.h just
like any other header file without having to include every header file
(and without having to include C++ specific aspects of the now all.h).

This PR supersedes #448.

Now diablo.h is treated in the same way as all other header files of
Source, as it only contains the declarations of global variables and
functions of diablo.cpp.

Besides consistency, this also enables mods to include diablo.h just
like any other header file without having to include every header file
(and without having to include C++ specific aspects of the now all.h).
@mewmew mewmew changed the title Now diablo.h is treated in the same way as all other header files of add all.h and use in place of diablo.h Feb 14, 2020
@mewmew
Copy link
Contributor Author

mewmew commented Feb 14, 2020

Note: this change is useful for mod developers as it enables linking against DevilutionX directly from C, thus increasing interoperability. Further background on modability is given in #447 (comment)

@AJenbo
Copy link
Member

AJenbo commented Feb 14, 2020

Didn't we decide to flip this and diasurgical/devilution#1848 ?

I can try and get it done this weekend. Here are comparisons of build times on the various CI before and after the PR:

  Before After Diff
Switch 00:04:54 00:04:44 -00:00:10
Unit tests 00:02:26 00:02:28 00:00:02
Linux 32bit 00:02:19 00:02:10 -00:00:09
Amiga m68k 00:00:59 00:00:49 -00:00:10
Linux SDL1 00:01:40 00:02:24 00:00:44
Linux 00:01:40 00:02:11 00:00:31
Windows GCC 00:01:52 00:02:32 00:00:40
Mac 10.3 00:03:29 00:03:26 -00:00:03
VS 2017 00:03:33 00:02:53 -00:00:40

@AJenbo
Copy link
Member

AJenbo commented Feb 14, 2020

Gave it a stab, but #1848 will require us to first remove miniwin (just one file left)

@AJenbo
Copy link
Member

AJenbo commented Feb 14, 2020

Moved to upstream as it makes more sense there, I think the different build times are down to randomly landing on different build servers.

@AJenbo AJenbo closed this Feb 14, 2020
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.

2 participants