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

Redstone fix #1461

Merged
merged 6 commits into from Sep 27, 2014

Conversation

Projects
None yet
3 participants
@worktycho
Member

worktycho commented Sep 27, 2014

Fixes #1460
Fixes #1458

@tigerw

This comment has been minimized.

Show comment
Hide comment
@tigerw

tigerw Sep 27, 2014

Member

The *.inc file is still not good though.

Member

tigerw commented Sep 27, 2014

The *.inc file is still not good though.

@tigerw

This comment has been minimized.

Show comment
Hide comment
@tigerw

tigerw Sep 27, 2014

Member

Merging as hotfix, if that is the correct term.

Member

tigerw commented Sep 27, 2014

Merging as hotfix, if that is the correct term.

tigerw added a commit that referenced this pull request Sep 27, 2014

@tigerw tigerw merged commit 28ebe11 into master Sep 27, 2014

1 check passed

continuous-integration/travis-ci The Travis CI build passed
Details

@tigerw tigerw deleted the RedstoneFix branch Sep 27, 2014

@madmaxoft

This comment has been minimized.

Show comment
Hide comment
@madmaxoft

madmaxoft Sep 27, 2014

Member

Why don't you use the approach used by all the other simulators - they each have a virtual method that creates the per-chunk data, and the chunk's constructor calls this method, so it always has a valid pointer to the simulator data. Or is there a problem with this and the splitting?

We could even make that method abstract within cSimulator itself, so each and every simulator has to override it.

Member

madmaxoft commented Sep 27, 2014

Why don't you use the approach used by all the other simulators - they each have a virtual method that creates the per-chunk data, and the chunk's constructor calls this method, so it always has a valid pointer to the simulator data. Or is there a problem with this and the splitting?

We could even make that method abstract within cSimulator itself, so each and every simulator has to override it.

@worktycho

This comment has been minimized.

Show comment
Hide comment
@worktycho

worktycho Sep 27, 2014

Member

The reason I didn't do that is that this is was hotfix and I didn't know about the factory method. I'm happy to rewrite it to use the factory method pattern.

Member

worktycho commented Sep 27, 2014

The reason I didn't do that is that this is was hotfix and I didn't know about the factory method. I'm happy to rewrite it to use the factory method pattern.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment