search

calref / cboe

Classic Blades of Exile
http://spiderwebforums.ipbhost.com/index.php?/forum/12-blades-of-exile/
Other
167 stars 41 forks source link

Trying to fix all of the builds #352

Closed NQNStudios closed 1 month ago

NQNStudios commented 1 month ago

For now, I can't figure out why, but my fork won't run the CI actions. So, I don't know if this is going to work, but I've tried adding a dependency to install-deps.bat and we'll see what happens.

CelticMinstrel commented 1 month ago

What was the reason to include boost/filesystem.hpp?

Unrelated, but it looks like the MacOS checks are never going to run because GitHub removed support for the version they want to run on… :(

NQNStudios commented 1 month ago

The reason to include filesystem.hpp (as far as I understand it) is that Visual Studio handles recursive includes differently than gcc does... I think.

I think for gcc, if file 1 includes file 2 which includes file 3, then file 1 can reference names defined in file 3. I think for Visual Studio's compiler, file 1 needs to include file 3 directly to get those names.

I could be wrong about why, but adding the includes definitely fixes the windows build.

CelticMinstrel commented 1 month ago

That doesn't make sense for reasoning… but adding them alongside operations.hpp may be redundant, I think? That is to say, if we're including filesystem.hpp it is quite likely redundant to also include filesystem/operations.hpp.

NQNStudios commented 1 month ago

If that's not the reason why Windows requires extra include statements, then the reason must be even more illogical and terrifying.

NQNStudios commented 1 month ago

Trying to pin the dependencies on Windows and Mac brought only heartache, so I reverted all my commits that were trying to do that (at least for now).

For some reason the visual studio builds failed this time, looks like with a network error when downloading packages? Trying to re-run those builds later should hopefully fix it because I think this commit is identical to the one that worked.

NQNStudios commented 1 month ago

I made a redo PR of this, minus the changes that broke it after the one build passed