Difference between revisions of "User talk:David corrales"

Jump to navigation Jump to search

User talk:David corrales (view source)

Revision as of 21:54, 1 August 2007

7,684 bytes added ,  21:54, 1 August 2007
Added a link to the proposal uml diagram
(Added a note for the table)
(Added a link to the proposal uml diagram)
 
(17 intermediate revisions by the same user not shown)
Line 11: Line 11:
----------------------------
----------------------------
* '''backends/plugins/posix/posix-provider.cpp'''
* '''backends/plugins/posix/posix-provider.cpp'''
1) getPlugins(): list all files in a directory using listDir(kListFilesOnly).
1) getPlugins(): list all files in a directory using listDir(kListFilesOnly), name(), path().


* '''backends/plugins/sdl/sdl-provider.cpp'''
* '''backends/plugins/sdl/sdl-provider.cpp'''
1) getPlugins(): list all files in a directory using listDir(kListFilesOnly).
1) getPlugins(): list all files in a directory using listDir(kListFilesOnly), name(), path().


* '''backends/plugins/win32/win32-provider.cpp'''
* '''backends/plugins/win32/win32-provider.cpp'''
1) getPlugins(): list all files in a directory using listDir(kListFilesOnly).
1) getPlugins(): list all files in a directory using listDir(kListFilesOnly), name(), path().


* '''backends/plugins/dc/dc-provider.cpp'''
* '''backends/plugins/dc/dc-provider.cpp'''
1) getPlugins(): list all files in a directory using listDir(kListFilesOnly).
1) getPlugins(): list all files in a directory using listDir(kListFilesOnly), name(), path().


* '''backends/platform/wince/CELauncherDialog.h'''
* '''backends/platform/wince/CELauncherDialog.h'''
1) automaticScanDirectory(FSNode): internally recurses files and dirs using listDir(kListFilesOnly) and listDir(kListDirectoriesOnly).
1) automaticScanDirectory(FSNode): internally recurses files and dirs using listDir(kListFilesOnly) and listDir(kListDirectoriesOnly). path().


* '''backends/fs/abstract-fs.h'''
* '''backends/fs/abstract-fs.h'''
Line 58: Line 58:
2) Engine_SWORD2_create(): listDir(kListAll). Calls Engine_SWORD2_detectGames().
2) Engine_SWORD2_create(): listDir(kListAll). Calls Engine_SWORD2_detectGames().


<strike>
* '''engines/scumm/scumm.cpp'''
* '''engines/scumm/scumm.cpp'''
-) removed the include and compilation went fine. Don't see any forward declarations.
-) removed the include and compilation went fine. Don't see any forward declarations.
</strike>


* '''engines/scumm/detection.cpp'''
* '''engines/scumm/detection.cpp'''
Line 74: Line 76:
6) Engine_SCUMM_create(): creates a FSList using listDir(kListFilesOnly).
6) Engine_SCUMM_create(): creates a FSList using listDir(kListFilesOnly).


<strike>
* '''engines/agos/agos.cpp'''
* '''engines/agos/agos.cpp'''
-) removed the include and compilation went fine. Don't see any forward declarations.
-) removed the include and compilation went fine. Don't see any forward declarations.
</strike>


<strike>
* '''engines/agi/agi.cpp'''
* '''engines/agi/agi.cpp'''
-) removed the include and compilation went fine. Don't see any forward declarations.
-) removed the include and compilation went fine. Don't see any forward declarations.
</strike>


* '''engines/agi/agi_v3.cpp'''
* '''engines/agi/agi_v3.cpp'''
Line 93: Line 99:
1) Engine_LURE_detectGames(FSList): isDirectory(), name().
1) Engine_LURE_detectGames(FSList): isDirectory(), name().


<strike>
* '''engines/gob/gob.cpp'''
* '''engines/gob/gob.cpp'''
-) removed the include and compilation went fine. Don't see any forward declarations.
-) removed the include and compilation went fine. Don't see any forward declarations.
</strike>


* '''engines/queen/queen.cpp'''
* '''engines/queen/queen.cpp'''
1) Engine_QUEEN_detectGames(FSList): isDirectory(), name().
1) Engine_QUEEN_detectGames(FSList): isDirectory(), name().


<strike>
* '''engines/cine/cine.cpp'''
* '''engines/cine/cine.cpp'''
-) removed the include and compilation went fine. Don't see any forward declarations.
-) removed the include and compilation went fine. Don't see any forward declarations.
</strike>


* '''gui/themebrowser.h|cpp'''
* '''gui/themebrowser.h|cpp'''
Line 117: Line 127:


* '''gui/options.cpp'''
* '''gui/options.cpp'''
1) handleCommand(...): create FSNodes, path().
1) handleCommand(...): create FSNodes, path(). There's a TODO here about the dir being writable.


* '''gui/launcher.cpp'''
* '''gui/launcher.cpp'''
1) handleCommand(...): create FSNodes, path().
1) handleCommand(...): create FSNodes, path(). There's a TODO here about checking that the game is in the given directory.


2) addGame(): listDir(kListAll), path().
2) addGame(): listDir(kListAll), path().
Line 180: Line 190:
|}
|}


== Common::File redesign discussion ==
== Common::File redesign discussion (old)==
While analizing the current state of the Common::File class, I've been noticing several important things that I feel, should be discussed.
While analizing the current state of the Common::File class, I've been noticing several important things that I feel, should be discussed.
First of all, is the relevance of breaking up this class (like the FilesystemNode) one into a different implementation for each architecture. This is the primary motivation for this section.
First of all, is the relevance of breaking up this class (like the FilesystemNode) into different implementations for each architecture. This is the primary motivation for this section.


=== External references ===
=== External references ===
Line 272: Line 282:
=== New methods ===
=== New methods ===
Adding new methods to the existing Common::File class wouldn't have an impact, unless it redefines or changes the signature of the old ones. Instead of breaking up said class, it could be cleaned up and extended with new functionality.
Adding new methods to the existing Common::File class wouldn't have an impact, unless it redefines or changes the signature of the old ones. Instead of breaking up said class, it could be cleaned up and extended with new functionality.
== New Common::File redesign proposal and implementation ==
After being involved in the project for more time, I analyzed the current design and created the following proposal to address the Common::File class.
Reading through the cpp file, I discovered that most discrepancies occur in the redefinition of standard I/O manipulation functions (e.g. fopen, ftell, fseek and so on) in 3 platforms: ds, ps2 and symbian. There's some #ifdef's around, but the algorithms are 99% the same for all platforms, so a separate implementation for each architecture is not needed and instead, would result in unnecessary code duplication.
Given this situation the template pattern looked like a really good alternative: create a base file class that implements the algorithm but allows subclasses to choose which method they want for fopen, fclose, etc calls. This is done through protected functions named _fopen, _close, etc.
Additionally, the current Common::File class is mixing responsibilities that go beyond a regular file class. For example, the whole default directory structure (contained in a hash map for quick access) is beyond the scope of a regular file manipulation object. The objective is then to convert Common::File into a wrapper for BaseFile, just like Common:FsNode is around AbstractFilesystemNode.
The responsibilities per function would be:
{| border="1" cellpadding="5" cellspacing="0"
!Function
!BaseFile
!Common::File
|-
|void clearIOFailed
|Clear the last I/O error flag
|Checks + Call the base class
|-
|virtual void close
|Close the open file
|Checks + Call the base class
|-
|bool eof
|Whether the seek pointer is at the EOF
|Checks + Call the base class
|-
|bool eos
|Whether the seek pointer is at the EOS
|Checks + Call the base class
|-
|bool ioFailed
|Whether the last I/O failed
|Checks + Call the base class
|-
|const char *name
|Name of the stream
|Checks + Call the base class
|-
|bool isOpen
|Whether the file is open
|Checks + Call the base class
|-
|virtual bool open
|Open a given filepath
|Use the default paths to locate the filepath, then call the base class
|-
|uint32 pos
|Position of the seek pointer within the file
|Checks + Call the base class
|-
|uint32 read
|Reads a chunk from a file
|Checks + Call the base class
|-
|virtual bool remove
|Remove a file
|Checks + Call the base class
|-
|void seek
|Move the seek pointer
|Checks + Call the base class
|-
|uint32 size
|Size of the file
|Checks + Call the base class
|-
|void addDefaultDirectory
| -
|Add a default search path
|-
|void addDefaultDirectoryRecursive
| -
|Recursive method of addDefaultDirectory
|-
|void resetDefaultDirectories
| -
|Clear default directories
|-
|bool exists
| -
|Whether the file exists. Uses Common:FSNode
|-
|}
The wrapper will handle the whole default search path functionality plus checks and maybe some caching for the BaseFile object, again, just like Common::FsNode does.
I did not list the write functions here, since a while ago I recall a discussion that implied removing the write abilities from the Common::File class. This of course needs to be defined clearly.
Another issue to be resolved is BaseScummFile inheriting Common::File. Max told me it was possible to code BaseScummFile as a wrapper instead of an inherited class, so that could be a possible solution.
The following diagram shows the class structure:
[[Image:File.png]]


== Current Filesystem API implementation diagrams==
== Current Filesystem API implementation diagrams==
[[Image:Fs1.png]]
[[Image:Fs1.png]]


== Filesystem Node Redesign proposal ==
== Filesystem Node Redesign Progress ==
[[Image:Fs2.png]]
[[Image:Fs2.png]]


'''Update 01/04''': added the FilesystemFactoryMaker class.
'''Update 01/04''': added the FilesystemFactoryMaker class.


== Common::File Redesign proposal ==
'''Update 05/06''': added the new methods ''exists(), isReadable(), isWritable()''.
 
'''Update 19/06''': removed isValid() and added the hidden parameter to getChildren()''.
 
== Common::File Redesign proposal (old)==
[[Image:Fs3.png]]
[[Image:Fs3.png]]


Line 290: Line 399:




'''exists(path) -> bool''': true if the file exists. '''Implemented'''


'''split(path) -> head, tail''': split the pathname path into a pair, (head, tail) where tail is the last pathname component and head is everything leading up to that. (python os.path)
'''split(path) -> head, tail''': split the pathname path into a pair, (head, tail) where tail is the last pathname component and head is everything leading up to that. (python os.path)
Line 299: Line 409:
'''isAbsolute(path) -> bool''': return True if path is an absolute pathname (begins with a slash). (python os.path)
'''isAbsolute(path) -> bool''': return True if path is an absolute pathname (begins with a slash). (python os.path)


'''isReadable(path) -> bool''': true if the file can be read.
'''isReadable(path) -> bool''': true if the file can be read. '''Implemented'''


'''isWriteable(path) -> bool''': true if the file can be written.
'''isWritable(path) -> bool''': true if the file can be written. '''Implemented'''


'''count(dir) -> int''': returns the total number of directories and files in the directory. (Qt)
'''count(dir) -> int''': returns the total number of directories and files in the directory. (Qt)


'''listDir(list, mode, filter) -> list''': lists the dir, with the given mode and filter. ''This function is already present. The idea is to augment it with filters''
'''getChildren(list, mode, hidden) -> list''': lists the dir, with the given mode and filter. ''This function is already present. The idea is to augment it with filters''. '''Added a filter to include hidden files'''


'''isHidden(path) -> bool''': tests whether the file pathname is a hidden file. (Java)
'''isHidden(path) -> bool''': tests whether the file pathname is a hidden file. (Java)


'''getCanonicalPath(path) ->  path''': returns the canonical pathname string of the abstract pathname. (Java)
'''getCanonicalPath(path) ->  path''': returns the canonical pathname string of the abstract pathname. (Java)
'''isValid() -> bool''': as per Max's instruction, this method was removed from the FilesystemNode class. It wasn't a heavily used method and its semantics weren't clear. It is now possible to use much more rich checks as ''(exists && isReadable)'' for example. '''Removed'''
'''Update 05/06''': marked some methods as implemented.
'''Update 19/06''': added the isValid() method as removed. Also updated getChildren().
== Savefile manager information ==
Due to request from Max, I've started working on the savefile manager also. Although this isn't technically part of the GSoC project, it's closely related and in need of work.
This section deals with information about the current state of affairs concerning the savefile manager class.
For more information about what needs to be done, one can visit http://wiki.scummvm.org/index.php/User:Fingolfin#TODO or http://wiki.scummvm.org/index.php/TODO#Savegames.
=== Save game procedure for the Scumm engine ===
#Click on the save button. (dialogs.cpp: save())
#Set the listed files from the list provided by generateSavegameList(). (dialogs.cpp)
##Mark the flags of the available savefiles using listSavegames(). (default-saves.cpp)
##For the available saves call getSavegameName(index, name). (Reads the user input name from the header) (saveload.cpp)
###Call makeSavegameName() to create a default filename.
####Creates a filename with "game|type|slot" format. Game is given by _targetName.
###Try to open the generated filename with openForLoading(filename).
###If 2.2.2 fails, return. Else continue loading the header, etc...
##Return the list of user input names from 2.2.
#Run the dialog and catch an index and a name for the savefile.
#Request a save file using requestSave() with the given index and file description.
=== Save file name format strings for all engines ===
*agi:
:sprintf(saveLoadSlot, "%s.%.3d", _targetName.c_str(), num + _firstSlot);
*agos:
:if (getGameId() == GID_DIMP) {
::sprintf(buf, "dimp.sav");
:} else if (getGameType() == GType_PP) {
::sprintf(buf, "swampy.sav");
:} else if (getGameType() == GType_FF) {
::sprintf(buf, "feeble.%.3d", slot);
:} else if (getGameType() == GType_SIMON2) {
::sprintf(buf, "simon2.%.3d", slot);
:} else if (getGameType() == GType_SIMON1) {
::sprintf(buf, "simon1.%.3d", slot);
:} else if (getGameType() == GType_WW) {
::sprintf(buf, "waxworks.%.3d", slot);
:} else if (getGameType() == GType_ELVIRA2) {
::sprintf(buf, "elvira2.%.3d", slot);
:} else if (getGameType() == GType_ELVIRA1) {
::sprintf(buf, "elvira1.%.3d", slot);
:}
*cine:
:snprintf(tmp, 80, "%s.dir", _targetName.c_str());
*gob:
:snprintf(slotBase, 3, "%02d", slot);
*kyra:
:sprintf(saveLoadSlot, "%s.%.3d", _targetName.c_str(), num + _firstSlot);
*lure:
:sprintf(buffer, "lure.%.3d", slotNumber);
*parallaction:
:sprintf(filename, "game.%i", slot);
*queen:
:strcpy(buf, "queen.asd");
:sprintf(buf, "queen.s%02d", slot);
*saga:
:sprintf(name, "%s.s%02d", _targetName.c_str(), slotNumber);
*scumm:
:sprintf(out, "%s.%c%.2d", _targetName.c_str(), temporary ? 'c' : 's', slot);
*sky:
:strcpy(fName, "SKY-VM-CD.ASD");
:sprintf(fName, "SKY-VM%03d.ASD", SkyEngine::_systemVars.gameVersion);
*sword1:
:_saveFileMan->openForLoading("SAVEGAME.INF");
:sprintf(fName, "SAVEGAME.%03d", slot);
*sword2:
:snprintf(buf, sizeof(buf), "%s.%.3d", _targetName.c_str(), slotNo);
*touche:
:snprintf(dst, len, "%s.%d", _targetName.c_str(), num);


== Misc ==
== Misc ==
Line 318: Line 504:
* a not yet existing but valid path to a file/dir (which is neither readable nor writable, I guess).
* a not yet existing but valid path to a file/dir (which is neither readable nor writable, I guess).
Well, that's not really a brilliant observation ; I just wanted to make sure this will be considered in the design process [[User:Fingolfin|Fingolfin]] 00:22, 24 March 2007 (CET)
Well, that's not really a brilliant observation ; I just wanted to make sure this will be considered in the design process [[User:Fingolfin|Fingolfin]] 00:22, 24 March 2007 (CET)
'''Update 19/06''': It is now possible to discern all these cases correctly using exists(), isReadable() and isWritable().




Another thing: look at <code>engines/scumm/plugin.cpp</code> and the <code>searchFSNode()</code> function in there. It can be used to look for a given file(name) within a directory (ignoring case). It's a bit like getChild() only that it ignores the file name case, and is only used to check for the presence of a given file. This is a commonly used function, I think. Might be worse taking this into consideration for the FS API redesign  [[User:Fingolfin|Fingolfin]] 21:41, 3 April 2007 (CEST)
Another thing: look at <code>engines/scumm/plugin.cpp</code> and the <code>searchFSNode()</code> function in there. It can be used to look for a given file(name) within a directory (ignoring case). It's a bit like getChild() only that it ignores the file name case, and is only used to check for the presence of a given file. This is a commonly used function, I think. Might be worse taking this into consideration for the FS API redesign  [[User:Fingolfin|Fingolfin]] 21:41, 3 April 2007 (CEST)
'''Update 08/07''': The lookupFile() function in fs.h/cpp can be used to supply this functionality. It uses wildcards for matching filenames.
Retrieved from "https://wiki.scummvm.org/index.php?title=Special:MobileDiff/6253...7082"

Navigation menu