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

ULTIMA8: Remove custom memory manager from engine #2227

Merged
merged 2 commits into from May 9, 2020

Conversation

@OMGPizzaGuy
Copy link
Contributor

OMGPizzaGuy commented May 7, 2020

ULTIMA8: Remove custom memory manager from engine

Back in 2005 I added custom memory allocation to Pentagram because I thought it would be a cool project (it was), and maybe it would improve performance on older systems (unlikely).
With the inclusion of the Ultima 8 engine in ScummVM this code doesn't belong in the engine, and I would be more comfortable an active developer adding performance optimizations based on actual requirements rather than my unfounded speculations.

Question: Should pent_include.cpp be removed now since it contains only the namespace stubs?

@mduggan
Copy link
Contributor

mduggan commented May 8, 2020

Thanks! I had been eyeing that code and wondering if it was better to remove, so glad you thought the same.

I think yeah, pent_include.cpp can be removed entirely now. If you want to add that to this PR that'd be great, otherwise I'm happy to do it.

@mduggan
Copy link
Contributor

mduggan commented May 9, 2020

Cool, thanks!

@mduggan mduggan merged commit df3e9e3 into scummvm:master May 9, 2020
2 checks passed
2 checks passed
Codacy/PR Quality Review Up to standards. A positive pull request.
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details
whiterandrek added a commit to whiterandrek/scummvm that referenced this pull request May 9, 2020
* ULTIMA8: Remove custom memory manager from engine
* ULTIMA8: Remove pent_include.cpp as it is empty
@OMGPizzaGuy OMGPizzaGuy deleted the OMGPizzaGuy:u8_remove_memory_manager branch May 9, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Linked issues

Successfully merging this pull request may close these issues.

None yet

2 participants
You can’t perform that action at this time.