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

ULTIMA: Ultima games engine #2025

Merged
merged 524 commits into from Feb 1, 2020
Merged

ULTIMA: Ultima games engine #2025

merged 524 commits into from Feb 1, 2020

Conversation

@dreammaster
Copy link
Member

dreammaster commented Jan 27, 2020

I'm at a stage where I'm happy enough with the Ultima engine that I've been working on to open it for review for inclusion into master. This engine currently consists of work for three games in the Ultima series.

  1. Ultima VIII - Pagan.
    This is based on the Pentagram project. I've finally got music playback working, and am about to start a full playthrough test of the game to may sure everything in the engine still works, and the game is completeable.

  2. Ultima VI - The False Prophet
    This is based on the Nuvie project. I added two game detection entries, one for Nuvie's "classic" mode, and another that turns on the Nuvie enhancements like Ultima 7 style gumps and a full-screen map. I haven't really tested it much beyond the game starting up, the initial fight, and moving around the castle a bit. But everything seems to work. One downside is that I had to include the entirety of the Lua interpreter that Nuvie used in order for the cutscenes and various in-game monster AI and combat calculations to work. So testing it further, and seeing whether I can get it to use the common Lua code is going on a back burner until I've finished testing Pagan.

  3. Ultima 1
    This is an in progress work based on my decompilation of the first Ultima game (not counting Akalabeth). Most of the in-game towns/castles, overworld, and dungeons are supported. It's just missing the outer space sim section, and the endgame bossfight. Further work on this will wait until the other two Ultima games are done.

@dreammaster dreammaster force-pushed the dreammaster:ultima branch from db6e757 to 71cf1e8 Jan 27, 2020
@sluicebox

This comment has been minimized.

Copy link
Member

sluicebox commented Jan 27, 2020

I would have clicked an emoji reaction but they don't have one for "jaw dropped like at the end of Parasite"

@sev-
sev- approved these changes Jan 27, 2020
Copy link
Member

sev- left a comment

Humongous work.

There is currently one problem with transparent_surface.h.

I could help you with fixning Lua, and reusing code in the Common. Let's discuss it on Discord.

Approved for the merge

*/
uint32 _palette[256];
bool _paletteSet;
protected:

This comment has been minimized.

Copy link
@sev-

sev- Jan 27, 2020

Member

This breaks at least Director engine. Why clip() this declared protected now?

This comment has been minimized.

Copy link
@dreammaster

dreammaster Jan 27, 2020

Author Member

Whoops, sorry. I'd gotten so used to just compiling with only the engine enabled, I forgot to ensure that my changes to the core didn't break anything else. I'll do an interactive rebase this evening and fix it in the history.

This comment has been minimized.

Copy link
@dreammaster

dreammaster Jan 27, 2020

Author Member

Regarding lua, any help would be welcome. I have a branch ultima6_lua on my github repository that has a single commit at the head where I was experimenting with extensions to the core Lua to re-enable proper file loading again. That could be a good starting point for any experiments.

This comment has been minimized.

Copy link
@dreammaster

dreammaster Jan 27, 2020

Author Member

The branch is a bit out of date though, so I'll also rebase it against the current ultima branch tonight.

@bluegr

This comment has been minimized.

Copy link
Member

bluegr commented Jan 27, 2020

Fantastic work, well done!

Some questions / observations:

  • I see no mention of Exult (Ultima 7). Is that considered for inclusion at some point in the future?
  • I'm a bit confused with the current folder structure. In particular:
  1. there's a folder called shared/core, a shared/engine and a shared/std. What are the differences?
  2. what kind of code is contained in shared/early?
  3. there's also a folder called ultima0. What kind of code does it contain?
  • What kind of issues were you having with the common LUA code? We use LUA 5.2.3, whereas nuvie used 5.1.3, which are quite similar
  • As a neat side project, the same engine used for Ultima 8 is also used in Crusader: No Remorse, and Crusader: No Regret. I suppose that's still in early development, as there's only a stub in ultima/ultima8/games/remorse_game.*
@yuv422

This comment has been minimized.

Copy link
Member

yuv422 commented Jan 27, 2020

Hey,
I'm still not sure how I feel moving the Nuvie project into SummVM. I've been thinking about it a bunch recently. I think it's mainly due to losing the Nuvie project as an entity and for it to just become another game that is supported by the ScummVM project. :(

@sev-

This comment has been minimized.

Copy link
Member

sev- commented Jan 27, 2020

Shall we withstand from the merging? Actually, the merge could help to bring more movement and fresh blood to Nuvie, and make it highly portable. You are more than welcome to join us if we merge.

@dreammaster

This comment has been minimized.

Copy link
Member Author

dreammaster commented Jan 28, 2020

Hey @yuv422 . Good to hear that you're still alive. Obviously I initially felt upset at hearing your doubts now, after all the work I did.. particularly given that work on Nuvie hadn't been done for a while, and I tried to contact you both directly, as well as via Dominus Dragon previously to see if the project was still active. However, on reflection, I calmed down and remembered that projects like this are a labor of love, and it's best to go with whatever place of development you're most happy with if you intend to keep working on Nuvie.

One possibility is that I'm presuming that the main Ultima VI is "done" and the game is completeable, and it's just work to be done on Savage Empire and Martian Dreams. In which case, the engine would still be merged in, but we'd refrain from doing any significant development on it beyond getting it stable and supported as is. That way, players could choose to use either ScummVM or Nuvie as they see fit, And if/when Nuvie, still as a separate project, ever got support done for the other games and you considered it "done", I could reintegrate the updated codebase into ScummVM at that point again; hopefully quicker the next time due to the foundation already have been laid by my previous conversion.

But if you do decide to carry on future development directly in ScummVM, we'd be very happy, and as Sev said, it might encourage further contributions. Not to mention ScummVM's portability.

@dreammaster dreammaster force-pushed the dreammaster:ultima branch from 71cf1e8 to 8f400e4 Jan 28, 2020
@yuv422

This comment has been minimized.

Copy link
Member

yuv422 commented Jan 28, 2020

@dreammaster I think this could work. I also think that scummvm could bring lots of benefits to Nuvie. I just don't really want to see Nuvie die as a project. Like you said I've spend a large amount of time with the project and it holds a special place in my heart. :) Could we maybe keep the Nuvie name for the engine in scummvm and maybe a nuvie logo splash screen :)

It would be great to see more progress on the engine. Especially supporting MD and SE.

I'll ask the other Nuvie team members to get their thoughts on merging with ScummVM.

@dreammaster

This comment has been minimized.

Copy link
Member Author

dreammaster commented Jan 28, 2020

Could we maybe keep the Nuvie name for the engine in scummvm and maybe a nuvie logo splash screen :)

That could work. Generally with the other projects previously merged in, it's just been credits in the credits.pl file, and on wiki.scummvm.org.. But it would be pretty easy to add a splash screen for Nuvie as the sub-engine starts up; it would just need someone with a suitable artistic flair to draw a suitable screen. :)

@dreammaster dreammaster force-pushed the dreammaster:ultima branch from 846bbba to 27f9bde Jan 28, 2020
@bluegr

This comment has been minimized.

Copy link
Member

bluegr commented Jan 28, 2020

@yuv422: as mentioned , merging Nuvie with ScummVM would bring new life to the project, and more portability, which is what happened in the past with the FreeSCI, Sarien, TrollVM, reinherit and z-vision projects, among others. Check this article for more insights:

https://arstechnica.com/gaming/2012/01/maniac-tentacle-mindbenders-of-atlantis-how-scummvm-kept-adventure-gaming-alive/3/

So... merging your work definitely won't make it just another game engine. It'll benefit from the common code we got, and ScummVM's portability. And as mentioned, you're always free to join us, and work not only with Nuvie, but with other engines too :)

As for the splash screen: do you feel that this would give any extra recognition to Nuvie? Will Nuvie coexist with ScummVM in the future?

Of course, all authors and references to the engine will be added to our credits:
https://www.scummvm.org/credits/#code_contrib

@yuv422

This comment has been minimized.

Copy link
Member

yuv422 commented Jan 28, 2020

As for the splash screen: do you feel that this would give any extra recognition to Nuvie? Will Nuvie coexist with ScummVM in the future?

Yeah I don't think there is much point in maintaining both versions going forward. But I would like to keep the Nuvie name going. Which I think the engine splash would accomplish.

Copy link
Member

sev- left a comment

As for the splash screen: do you feel that this would give any extra recognition to Nuvie? Will Nuvie coexist with ScummVM in the future?

Yeah I don't think there is much point in maintaining both versions going forward. But I would like to keep the Nuvie name going. Which I think the engine splash would accomplish.

Yes, this is a good and feasible idea. As of the naming of the engine, I think, it could also be easily accomplished by renaming ultima/ultima6 to ultima/nuvie.

And of course, even if you have no time/desire to join the ScummVM team, we will incorporate the whole credits section, so due diligence is established.

Also, while we have you here ;) Dreammaster needed any disassemblies which you probably still have, so he could complete the support for other sub-engines.

# This file is included from the main "configure" script
# add_engine [name] [desc] [build-by-default] [subengines] [base games] [deps]
add_engine ultima "Ultima games" no "ultima1 ultima6 ultima8"
add_engine ultima1 "Ultima I" no "" "" "highres 16bit"

This comment has been minimized.

Copy link
@sev-

sev- Jan 28, 2020

Member

quick question. What is the reason for having individual games as the targets? I mean, usually we do it only when there are some difference in hardware requirements like resolution.

Also, is ultima1 really highres and 16bit?

This comment has been minimized.

Copy link
@dreammaster

dreammaster Jan 28, 2020

Author Member

Also, while we have you here ;) Dreammaster needed any disassemblies which you probably still have, so he could complete the support for other sub-engines.

I hope you don't take it that I'm stepping on your toes, so to speak. If you intend to keep working on the games, you'd remain the primary developer. And goodness knows I have plenty to get on with for the other games of the series. But it would be a good idea, if you're willing, to make a copy of your disassemblies available to us, Dominus, and any others, just for archival purposes if nothing else.

This comment has been minimized.

Copy link
@dreammaster

dreammaster Jan 28, 2020

Author Member

quick question. What is the reason for having individual games as the targets? I mean, usually we do it only when there are some difference in hardware requirements like resolution.

Now that I think of it, that's actually a good question. My initial though was that the sub-engines are more or less full blown engines in their own right, just consolidated together in one overall engine, with some shared code for event handling, saves, screen, etc. This is moreso than engines like TsAGE where although the different games have their own hardcoded logic, they share more of the engine in common. But in retrospect, you're right.. the sub-engines aren't so big that I can see a particular need or advantage for having them individually disable-able. I'll go ahead and remove them. This will also simplify the module.mk

Also, is ultima1 really highres and 16bit?

Not in classic mode, but it also has even-more-in-progress code for creating an enhanced mode that will use the Ultima 6 tileset. Though if there won't be individual sub-engines listed anymore, it's a moot point.

This comment has been minimized.

Copy link
@yuv422

yuv422 Jan 29, 2020

Member

@sev- Yes I've got some IDA dissasseblies for U6 and MD. Where should I put them?

This comment has been minimized.

Copy link
@sev-

sev- Jan 29, 2020

Member

Please e-mail them to me at sev@scummvm.org or upload to thing like Google Drive and share the link

}

virtual const char *getOriginalCopyright() const override {
return "Ultima games (C) Origin Systems Inc.";

This comment has been minimized.

Copy link
@sev-

sev- Jan 28, 2020

Member

please add copyright years

This comment has been minimized.

Copy link
@dreammaster

dreammaster Jan 28, 2020

Author Member

Okay, I'll do that tonight.

shared/maps/creature.o
endif

ifdef ENABLE_ULTIMA1

This comment has been minimized.

Copy link
@sev-

sev- Jan 28, 2020

Member

Why the same ifdef is used? Maybe it makes sense to merge two lists?

This comment has been minimized.

Copy link
@dreammaster

dreammaster Jan 28, 2020

Author Member

Essentially the first block is for the "shared" code that's currently only used by Ultima 1, but will likely be reused for Ultima 2 and other early games as well. So semantically it just seemed cleaner to have it as a separate block after the shared files that all the sub-engines need, rather than just merging it into the Ultima 1 block.

namespace Ultima {
namespace Shared {

int String::indexOf(char c) const {

This comment has been minimized.

Copy link
@sev-

sev- Jan 28, 2020

Member

I think, these deserve to be moved to common/str.h

This comment has been minimized.

Copy link
@dreammaster

dreammaster Jan 28, 2020

Author Member

Hmm.. I think you're right. Goodness knows that this isn't the first time I've had to implement similar functions in other engines, so they might as well go in the core to be available for other usage

@sev-

This comment has been minimized.

Copy link
Member

sev- commented Jan 28, 2020

Compilation is still failing on Mac:

./engines/ultima/ultima6/files/nuvie_io_file.h:65:19: error: no member named '__1' in namespace 'Ultima::std'; did you mean '::std::__1'?
                return _file != nullptr;
                                ^~~~~~~

The thing is that nullptr is an object in MacOS X, and that leads to tons of side effects. Here you have a nice and nasty clash of the namespace std in shared/std/*.h. I recommend renaming that to Std or something like this.

@sev-

This comment has been minimized.

Copy link
Member

sev- commented Jan 28, 2020

@dreammaster

This comment has been minimized.

Copy link
Member Author

dreammaster commented Jan 28, 2020

Compilation is still failing on Mac:

Darn. I thought I'd got all such problems fixed. I think your idea of changing std to Std has merit if it'll help avoid such clashes, and it'll be easy enough to do a global search and replace of existing namespace qualifiers in the code. Again, I'll take care of it tonight

@dreammaster

This comment has been minimized.

Copy link
Member Author

dreammaster commented Jan 28, 2020

Fantastic work, well done!

Thanks :)

Some questions / observations:

  • I see no mention of Exult (Ultima 7). Is that considered for inclusion at some point in the future?

It's definitely a possibility. For now, at least, Exult is still an active project, and is also complicated by needing to also support Exult Studio. So I'll likely end up working on the earlier games first after the current ones get officially supported. Maybe even in pull in xu4 as well.

  • I'm a bit confused with the current folder structure. In particular:
  1. there's a folder called shared/core, a shared/engine and a shared/std. What are the differences?

shared/engine is the subfolder for more directly core engine related stuff like the base engine class, the events manager, and so on.
shared/core is for more general purpose classes, like strings widgets, etc.
shared/std is for mockups/reimplementation of std library classes, so I didn't need to bulk changes to the codebase I was pulling in to use ScummVM's classes and methods directly, and potentially introduce errors.

  1. what kind of code is contained in shared/early?

shared/early is an interesting special case. Prior to doing the pull request, I wanted to bring all three Ultima games I'd worked on into a single branch. The shared/early was an attempt to keep some files separate from shared/core/ that would likely end up only being used by the earlier games, and not for u6+ onwards. This was only partially successful, since shared/core/ still ended up with some classes like Map and Message Target that U6 & U8 aren't using. At the point when I return to work on U1, I'll probably clean up the shared/ folder structure further

  1. there's also a folder called ultima0. What kind of code does it contain?

It contains more tentative/skeleton code for eventually supporting Akalabeth.

  • What kind of issues were you having with the common LUA code? We use LUA 5.2.3, whereas nuvie used 5.1.3, which are quite similar

I'm honestly not sure. By the time I had a proper look at the common lua code, and realised it was hardcoded for loading only a config.lua, I had already imported and converted Nuvie's Lua version, and had it working. I did tentatively try adding proper loading code to common lua, but when I still got a general error, I put further looking into it on a backburner. At Sev's request, I've wrapped the Lua access points in the engine with an ifdef for 'USE_COMMON_LUA', so it'll be easy to switch it over if you want to do any experiments.

Unfortunately, according to Strangerke, only wjp and/or fingolfin had the IDBs for the games, and they're not responding. So further support for them isn't likely. But there's so little code for the games, it wasn't worth the effort to remove it.

size_t rfind(char c, size_t pos = npos) const;

/** Find first character in the string matching the passed character */
size_t find_first_of(char c, size_t pos = 0) const;

This comment has been minimized.

Copy link
@bgK

bgK Jan 29, 2020

Member

The existing methods in this class use a camel case naming convention.

This comment has been minimized.

Copy link
@dreammaster

dreammaster Jan 29, 2020

Author Member

Good point. Those methods currently match the naming of methods from the C std::string class, but for ScummVM it'll be better for consistency to have them camelcase. Particularly since for both the current Ultima code as well as any future code we import that uses std::string, it will be pretty simple to do a global search and replace to correct the method names to the ScummVM version

@yuv422

This comment has been minimized.

Copy link
Member

yuv422 commented Jan 29, 2020

@yuv422 Is this your logo: http://nuvie.sourceforge.net/images/nuvie_logo.png ?

Yes that's the Nuvie logo. :)

@dreammaster

This comment has been minimized.

Copy link
Member Author

dreammaster commented Jan 29, 2020

@yuv422 Is this your logo: http://nuvie.sourceforge.net/images/nuvie_logo.png ?
Yes that's the Nuvie logo. :)

Maybe someone familiar with Lua programming could, post merge, change the Lua scripts for the game intro sequences to additionally load the image in the corner of the first shown screen. Both adding the image and updating the existing Lua scripts would both require updating ultima.dat, so it could be done at the same time.

@yuv422

This comment has been minimized.

Copy link
Member

yuv422 commented Jan 29, 2020

@yuv422 Is this your logo: http://nuvie.sourceforge.net/images/nuvie_logo.png ?
Yes that's the Nuvie logo. :)

Maybe someone familiar with Lua programming could, post merge, change the Lua scripts for the game intro sequences to additionally load the image in the corner of the first shown screen. Both adding the image and updating the existing Lua scripts would both require updating ultima.dat, so it could be done at the same time.

I can do this in another PR if you like.

@dreammaster

This comment has been minimized.

Copy link
Member Author

dreammaster commented Jan 30, 2020

I can do this in another PR if you like.

Thanks. Having the expert take care of it will make things easier :)

add_person("Paul Gilbert", "dreammaster", "");
add_person("Eric Fry", "yuv422", "(Nuvie)");

This comment has been minimized.

Copy link
@dreammaster

dreammaster Jan 31, 2020

Author Member

I added users with 100+ commits to either project, based on Openhub (thanks Lightkey for pointing out I use that to check contribution totals). If any of the users have publically known names, let me know and I'll flesh out their entries

@@ -434,7 +434,7 @@ void String::deleteChar(uint32 p) {
}

void String::erase(uint32 p, uint32 len) {
if (p == npos)
if (p == npos || len == 0)

This comment has been minimized.

Copy link
@dreammaster

dreammaster Jan 31, 2020

Author Member

Credit to yuv422 for noticing the erase problems

@yuv422

This comment has been minimized.

Copy link
Member

yuv422 commented Jan 31, 2020

image
The icons on the inventory view are broken

@yuv422

This comment has been minimized.

Copy link
Member

yuv422 commented Jan 31, 2020

Also the game crashes when you enter and exit the main Nuvie menu using the esc key.

Thread 0 Crashed:: Dispatch queue: com.apple.main-thread
0   scummvm                       	0x000000010155263c Ultima::Shared::EventsManager::getMousePos() const + 12 (events.h:337)
1   scummvm                       	0x0000000101552611 Ultima::Nuvie::Screen::get_mouse_location(int*, int*) + 33 (screen.cpp:1599)
2   scummvm                       	0x00000001014d7edd Ultima::Nuvie::GUI::Display() + 189 (gui.cpp:207)
3   scummvm                       	0x00000001014d8067 Ultima::Nuvie::GUI::CleanupDeletedWidgets(bool) + 343 (gui.cpp:166)
4   scummvm                       	0x00000001014d859a Ultima::Nuvie::GUI::HandleEvent(Common::Event*) + 1034 (gui.cpp:343)
5   scummvm                       	0x000000010146c3d7 Ultima::Nuvie::Events::update() + 183 (events.cpp:183)
6   scummvm                       	0x0000000101487ea1 Ultima::Nuvie::Game::play() + 129 (game.cpp:646)
7   scummvm                       	0x00000001015c1f87 Ultima::Nuvie::NuvieEngine::run() + 87 (nuvie.cpp:179)
8   scummvm                       	0x00000001011f8311 runGame(Plugin const*, OSystem&, Common::String const&) + 3361 (main.cpp:300)
9   scummvm                       	0x00000001011f6888 scummvm_main + 3000 (main.cpp:582)
10  scummvm                       	0x00000001011d8b06 main + 182 (posix-main.cpp:45)
11  libdyld.dylib                 	0x00007fff6dd823d5 start + 1
@dreammaster

This comment has been minimized.

Copy link
Member Author

dreammaster commented Feb 1, 2020

Also the game crashes when you enter and exit the main Nuvie menu using the esc key.

Fixed. There was an incorrect forced cast between GUI_CallBack and CallBack.

@dreammaster

This comment has been minimized.

Copy link
Member Author

dreammaster commented Feb 1, 2020

The icons on the inventory view are broken

There's something screwy with the buttons; likely a format or palette style mismatch between the colour32 array and the resulting ScumMVM surface pixel format. I'll have to look into it further post-merge

dreammaster added 27 commits Jan 28, 2020
Since ActorPathFinder::check_dir explicitly says it passes out
changes to rel, it makes sense to not discard any such changes
@dreammaster dreammaster force-pushed the dreammaster:ultima branch from 64d62df to b15ab9e Feb 1, 2020
@dreammaster dreammaster merged commit 893fa1c into scummvm:master Feb 1, 2020
0 of 2 checks passed
0 of 2 checks passed
Codacy/PR Quality Review Hang in there, Codacy is reviewing your Pull request.
Details
continuous-integration/travis-ci/pr The Travis CI build is in progress
Details
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

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