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

Added a path to script folder for require to work properly. #264

Draft
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

Zly-u
Copy link
Collaborator

@Zly-u Zly-u commented Aug 1, 2020

Still in WIP in some way, will try to figure out a better way to do it.
Meanwhile, you can suggest your ideas for what method would fit this better.

@Zly-u Zly-u linked an issue Aug 1, 2020 that may be closed by this pull request
6 tasks
@Morxemplum
Copy link
Collaborator

Don't link an issue if you're using a checklist. I wish GitHub was that sophisticated and could allow you to check off tasks using pull requests.

@Morxemplum Morxemplum removed a link to an issue Aug 1, 2020
6 tasks
@Zly-u
Copy link
Collaborator Author

Zly-u commented Aug 1, 2020

dunno how that is a problem since you can link several issues but okay i guess..?

@Zly-u Zly-u self-assigned this Aug 1, 2020
@Morxemplum
Copy link
Collaborator

Because if you close the issue then it's assumed you've completed the entire check list (which I don't think is the case here). You can only link issues, not the tasks contained within them.

@Zly-u
Copy link
Collaborator Author

Zly-u commented Aug 1, 2020

Uuuhhhh what. K whatever that just makes no sense to me.

@Morxemplum
Copy link
Collaborator

It doesn't, and again, I wish GitHub had this as a feature, but unfortunately we don't.

Copy link
Collaborator

@Morxemplum Morxemplum left a comment

Choose a reason for hiding this comment

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

Uhhhhhh. There's some good changes, but some others seem pretty sketchy, I'd like an explanation as to why some of those changes were made.

Comment on lines -9 to -11
#include "SSVOpenHexagon/Components/CWall.hpp"
#include "SSVOpenHexagon/Components/CCustomWallHandle.hpp"
#include "SSVOpenHexagon/Components/CCustomWall.hpp"
Copy link
Collaborator

Choose a reason for hiding this comment

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

Why are you removing this? There are custom wall functions defined here that need these headers.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

they are not even used in the include.

Comment on lines 1215 to +1221
addLuaFn("u_getAttemptRandomSeed", //
[this] { return rng.seed(); })
.doc(
"Obtain the current random seed, automatically generated at the "
"beginning of the level. `math.randomseed` is automatically "
"initialized with the result of this function at the beginning of "
"a level.");
[this]{ return rng.seed(); })
.doc(
"Obtain the current random seed, automatically generated at the "
"beginning of the level. `math.randomseed` is automatically "
"initialized with the result of this function at the beginning of "
"a level.");
Copy link
Collaborator

Choose a reason for hiding this comment

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

Be sure to stick to the .clang-format that Vee has defined for us.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

can you go away with it when the thing is god damn raw?

Comment on lines -1228 to 1241
try
{
try{
lua.executeCode("math.randomseed(u_getAttemptRandomSeed())");
}
catch(...)
{
catch(...){
ssvu::lo("HexagonGame::initLua")
<< "Failure to initialize Lua random generator seed\n";
<< "Failure to initialize Lua random generator seed\n";
}
Copy link
Collaborator

Choose a reason for hiding this comment

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

Same thing applies here.

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