calling class constructors#291
Conversation
jt-traub
left a comment
jt-traub
left a comment
There was a problem hiding this comment.
Please fix the issues with the teleport order and the changes to the unit tests which make the tests less useful.
jt-traub
left a comment
jt-traub
left a comment
There was a problem hiding this comment.
There are still changes to the unit tests which I feel are bad changes.
This code cannot have had any effect.
Always use new ARegion();
also refactored some constructors, especially for Order classes. making sure Order destructors are virtual. replacing empty destructors with default destructor where possible.
even if default values will be overwritten by Readin.
also address shellcheck warnings: * add quotes to prevent globbing and word splitting * Check exit codes directly, don't use test + $?
jt-traub
left a comment
jt-traub
left a comment
There was a problem hiding this comment.
This looks good to me now code-wise, but I still want to run it through my regression tests.
I am out of town at WorldCon through Sunday, so it will be a week until I get around to being able to do that. But I won't be doing anything else on the code in that timeframe either. Please don't merge until I have a chance to do that regression run.
jt-traub
left a comment
jt-traub
left a comment
There was a problem hiding this comment.
Tested with my regression suite and all passed.
Single issue was a slight bug with CastInvisibility which I pushed a small fix for to this PR.
2 participants
lots of
new Foo;that was better off beingnew Foo();