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

ZVISION: More complete engine implementation #532

Merged
merged 195 commits into from Dec 2, 2014
Merged

Conversation

@Marisa-Chan
Copy link
Contributor

Marisa-Chan commented Nov 17, 2014

No description provided.

@RichieSams

This comment has been minimized.

Copy link

RichieSams commented on 82348a7 Oct 22, 2013

What is the difference between this class and Control? The only thing I can find so far is SideFXType _type and it's being set in the constructor, but never used.

This comment has been minimized.

Copy link

bluegr replied Oct 31, 2013

I don't understand this. Why are you trying to rewrite the Control class into this class? Are there any actual benefits, apart from trying to remove inheritance?

This comment has been minimized.

Copy link
Owner Author

Marisa-Chan replied Nov 3, 2013

Basic different between control and sidefx is in interactions with user and type-specific. Control produce interactions and results, sidefx class produce only some results. action:kill and action:stop works only with sidefx objects. Additionally action:kill can interact with many sidefx objects like action:kill("ALL") or action:kill("AUDIO"). As well controls may use Mouse and Key interaction methods thats not needed in sidefx. In other words combine it in one class will cause many complication of code and simple graphical methods will be clogged by workarounds.

@RichieSams

This comment has been minimized.

Copy link

RichieSams commented on 2e0e9dc Oct 27, 2013

All my .scr files read as such: event:change_location(T,E,10,1139)

This comment has been minimized.

Copy link
Owner Author

Marisa-Chan replied Oct 27, 2013

This comment has been minimized.

Copy link

RichieSams replied Oct 27, 2013

Yea that's a line from a ZNem .scr file

This comment has been minimized.

Copy link
Owner Author

Marisa-Chan replied Oct 27, 2013

This comment has been minimized.

Copy link

RichieSams replied Oct 27, 2013

:( Kk. I need to look into sscanf anyway due to locale making reading floating-points wrong.

This comment has been minimized.

Copy link
Owner Author

Marisa-Chan replied Oct 27, 2013

@wjp
Copy link
Member

wjp commented Nov 17, 2014

I haven't gone throught the commits/code yet, but this looks like a lot has been improved. Nice work. Could you maybe summarize the current status of the engine? (Since there's no description on the pull request.)

@Marisa-Chan
Copy link
Contributor Author

Marisa-Chan commented Nov 17, 2014

Implemented all base code and functionality. Games should be completable but massive testing is needed.
But some features not implemented yet like using scummvm load/save/menu dialogs, dvd version support(add handle for video vob's), fan-made translations and another little things

next_loc.view = stream->readByte();
next_loc.offset = stream->readUint32LE() & 0x0000FFFF;

// What the fck, eos is not 'return pos >= size'

This comment has been minimized.

Copy link
@bluegr

bluegr Nov 17, 2014

Member

This sounds like a bug (and please remove the swear word). Perhaps the parent stream is not returning the EOS correctly

This comment has been minimized.

Copy link
@clone2727

clone2727 Nov 17, 2014

Contributor

No, it sounds like eos() behaving exactly as it should.

This comment has been minimized.

Copy link
@bluegr

bluegr Nov 18, 2014

Member

Is it? I thought that these two should be equivalent:
while (!stream->eos())
and
while (stream->pos() < stream->size())

if pos() >= size(), then the eos flag should be set

This comment has been minimized.

Copy link
@wjp
private:
ZVision *_engine;

struct script_scope {

This comment has been minimized.

Copy link
@bluegr

bluegr Nov 17, 2014

Member

CamelCase: script_scope -> scriptScope (same for the struct members)


PuzzleList *scope_queue; // For adding puzzles to queue
PuzzleList *exec_queue; // Switch to it when execute
PuzzleList _priv_queue_one;

This comment has been minimized.

Copy link
@bluegr

bluegr Nov 17, 2014

Member

Why do you use an underscore prefix here? We use underscore prefixes for global variables in ScummVM

This comment has been minimized.

Copy link
@clone2727

clone2727 Nov 17, 2014

Contributor

We don't use the underscore prefix for global variables. It's for class
variables. The struct should be consistent, though.

This comment has been minimized.

Copy link
@bluegr

bluegr Nov 18, 2014

Member

Yes, I should have said class member variables here. Here's the page with our code formatting conventions, esp. variable naming.
http://wiki.scummvm.org/index.php/Code_Formatting_Conventions#Naming

struct puzzle_ref {
Puzzle *puz;
script_scope *scope;
};

This comment has been minimized.

Copy link
@bluegr

bluegr Nov 17, 2014

Member

CamelCase again

// TODO: Implement FistControl
// TODO: Implement HotMovieControl
// TODO: Implement PaintControl
// TODO: Implement TilterControl

This comment has been minimized.

Copy link
@bluegr

bluegr Nov 17, 2014

Member

These have been implemented, so these TODOs can be removed


namespace ZVision {

DistortNode::DistortNode(ZVision *engine, uint32 key, int16 speed, float st_angl, float en_angl, float st_lin, float en_lin)

This comment has been minimized.

Copy link
@bluegr

bluegr Nov 17, 2014

Member

CamelCase

//int16 x;
//int16 y;
//uint16 w;
//uint16 h;

This comment has been minimized.

Copy link
@bluegr

bluegr Nov 17, 2014

Member

These should probably be removed now, since they are contained in the rect above

break;
}

// Crush each octet pair to a single octet with a simple cast

This comment has been minimized.

Copy link
@bluegr

bluegr Nov 17, 2014

Member

This doesn't look endian-safe... you should either use the endian-safe functions in endian.h, or add an if (SCUMMVM_BIG_ENDIAN) check

This comment has been minimized.

Copy link
@clone2727

clone2727 Nov 17, 2014

Contributor

Why doesn't it look endian safe? Looks fine to me.

This comment has been minimized.

Copy link
@bluegr

bluegr Nov 18, 2014

Member

Hm, yes, disregard my original post, the code below is OK

This comment has been minimized.

Copy link
@wjp

wjp Nov 18, 2014

Member

The comments and variable names are confusing, though. "asciiString" doesn't seem to be ASCII, and I'm not sure what "Crush each octet pair to a single octet with a simple cast" refers to.

This comment has been minimized.

Copy link
@RichieSams

RichieSams Nov 18, 2014

Contributor

That's my old comment. The text files assume UTF16. However, we don't currently support UTF16. My old code just naively truncated the uint16 to a single byte

#ifndef ZVISION_INVENTORY_MANAGER_H
#define ZVISION_INVENTORY_MANAGER_H
#ifndef ZVISION_WINKEY_H
#define ZVISION_WINKEY_H

This comment has been minimized.

Copy link
@bluegr

bluegr Nov 17, 2014

Member

ZVISION_WINKEY_H -> ZVISION_WIN_KEYS_H

@Marisa-Chan Marisa-Chan force-pushed the Marisa-Chan:zvision branch from 70ae3c5 to d5f7a1d Nov 20, 2014
@Marisa-Chan
Copy link
Contributor Author

Marisa-Chan commented Nov 20, 2014

Huge renamings applied. Maybe something is missed.


if (checkCode("MIKESPANTS")) {
_scriptManager->changeLocation('g', 'j', 't', 'm', 0);
}

This comment has been minimized.

Copy link
@bluegr

bluegr Nov 23, 2014

Member

Just to add to this list: there are 3 more cheats in ZGI - "EAT ME", "WHOAMI" and "HUISOK". Check here:
http://www.mrbillsadventureland.com/amusing/funthings/zorkF/zgiF.htm

This comment has been minimized.

Copy link
@Marisa-Chan

Marisa-Chan Dec 1, 2014

Author Contributor

Those cheats are scripted in gjcr.scr, hp1e.scr, uh1f.scr

This comment has been minimized.

Copy link
@bluegr

bluegr Dec 1, 2014

Member

It would be good to add a small comment about these, then

@bluegr
Copy link
Member

bluegr commented Dec 1, 2014

Overall, there are huge changes here, all major improvements, and almost all of the missing features have been implemented. Great work!

IMHO, this can be merged as-is, and any further development can continue in-tree. Some rough edges might need to be fixed here and there, but they're all minor issues.

Unless anyone objects, I'll merge this by tomorrow night (Tuesday, the 2nd of November).

@RichieSams
Copy link
Contributor

RichieSams commented Dec 1, 2014

I agree with @bluegr. Any further changes can be done in-branch. (Though I think you mean the 2nd of December :P)

I finally have a free week this week. So I intend to look further into the changes.

@bluegr
Copy link
Member

bluegr commented Dec 2, 2014

Merging as mentioned already. Great work, thanks! Further changes will be continued in-tree

bluegr added a commit that referenced this pull request Dec 2, 2014
ZVISION: More complete engine implementation
@bluegr bluegr merged commit 637102d into scummvm:master Dec 2, 2014
bluegr added a commit that referenced this pull request Dec 3, 2014
Thanks to marisa-chan for providing the location of those in pull
request #532
@clone2727

This comment has been minimized.

Copy link
Contributor

clone2727 commented on engines/zvision/subtitles.cpp in ac9b74d Jan 10, 2015

The bracket syntax is C99. Do all our platforms support this?

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

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