Bug Report: Graphics not masking

Collapse
X
 
  • Time
  • Show
Clear All
new posts
  • Blue Baron
    Adept
    • Apr 2011
    • 103

    Bug Report: Graphics not masking

    The 8x8, 8x16, and 16x16 tiles are not masking properly in windows because they have a white background, rather than a black one. I think whoever saves them probably needs to make sure that the paper color is black, since it probably cannot be seen because of the transparency.

    Also, ANGBAND_DIR_XRTA_FONT is still being initialized in init_stuff in main-win.c (lines 4873-4878). It should be either removed from here, like the ones for graphics and sounds were, or put in an if (!ANGBAND_DIR_XRTA_FONT) wrapper. Like the others were, the memory is being allocated twice, but only freed once.

    Also, would ticket #1461 be fixed by removing the line "file_delete(old_savefile);" from line 380 and putting it on line 402? (The exsiting .old file is not deleted until the .new file is confirmed to have been written, but before the existing save file is renamed .old.) You can remove the lines " else file_delete(old_savefile);" on lines 414 and 415, if you want the .old file to remain as a backup.

    For ticket #942, I'm curious what the OSX and SDL ports use for icons?

    Lastly, what does the "multiple save file handling " mentioned in other threads mean? I could not find a therad that talked about it.

    Edit: for the first part of ticket #1469, adding the line "case VK_CLEAR: ch = '5'; kp=TRUE; break;" in handle_keydown() works for me. (wParam is 12 for me when pressing numpad5 when numlock is not on.)

    Edit: For the second part of ticket #1469, the numpad keys are not handled in handle_keydown(), but WM_CHAR message are not generated for them, so they are not handled that way. ( I do not know where the plain numpad keypresses are handled.) Maybe in handle_keydown add lines like "case VK_NUMPAD7: if (mc||ma||ms) {ch = VK_NUMPAD7; kp = TRUE;} break;"
    Last edited by Blue Baron; June 19, 2011, 04:37.
  • myshkin
    Angband Devteam member
    • Apr 2007
    • 334

    #2
    Originally posted by Blue Baron
    Also, would ticket #1461 be fixed by removing the line "file_delete(old_savefile);" from line 380 and putting it on line 402? (The exsiting .old file is not deleted until the .new file is confirmed to have been written, but before the existing save file is renamed .old.) You can remove the lines " else file_delete(old_savefile);" on lines 414 and 415, if you want the .old file to remain as a backup.
    No. The initially reported problem was that people were manually making copies of their savefiles, and the game was deleting them. The game should not be using any fixed filenames as temporary files.

    Originally posted by Blue Baron
    For ticket #942, I'm curious what the OSX and SDL ports use for icons?
    Look at lib/xtra/icon/att*.png.

    Originally posted by Blue Baron
    Edit: for the first part of ticket #1469, adding the line "case VK_CLEAR: ch = '5'; kp=TRUE; break;" in handle_keydown() works for me. (wParam is 12 for me when pressing numpad5 when numlock is not on.)

    Edit: For the second part of ticket #1469, the numpad keys are not handled in handle_keydown(), but WM_CHAR message are not generated for them, so they are not handled that way. ( I do not know where the plain numpad keypresses are handled.) Maybe in handle_keydown add lines like "case VK_NUMPAD7: if (mc||ma||ms) {ch = VK_NUMPAD7; kp = TRUE;} break;"
    Strange. I guess the mapping of keypad 5 to clear does have some historical precedent. I know essentially nothing about the Windows port, and so will wait for someone else to comment, but this is quite encouraging.

    Comment

    • d_m
      Angband Devteam member
      • Aug 2008
      • 1517

      #3
      Blue Baron, you are my personal hero!

      In case anyone else is interested, I fixed the PNG tilesets by opening them in gimp, saving a copy as PNG, turning off "save background color" and "save color value for transparent pixels".

      For some reason (under WINE) I was able to walk/run/alter using the number pad with numlock on and off after applying your fix. Are you seeing problems still?
      linux->xterm->screen->pmacs

      Comment

      • Blue Baron
        Adept
        • Apr 2011
        • 103

        #4
        Originally posted by myshkin
        No. The initially reported problem was that people were manually making copies of their savefiles, and the game was deleting them. The game should not be using any fixed filenames as temporary files.
        so something like :
        Code:
        strnfmt(old_savefile, sizeof(old_savefile), "%s.old", path);
        while (file_exists(old_savefile) && (count++ < 100))
        {
            strnfmt(old_savefile, sizeof(old_savefile), "%s.%u%u.old", path, Rand_simple(32767), count);
        }
        except use the platform specific code where available? (file_exists has defines for platform specific code in z-file.c) err actually the Rand_simple function is defined in z-rand.h, but is not implemented. is there another random function that will not interfere with the game's rng?

        Originally posted by myshkin
        Look at lib/xtra/icon/att*.png.
        Ahh thanks I did not know they were there and was only looking in the src directory.

        Comment

        • myshkin
          Angband Devteam member
          • Apr 2007
          • 334

          #5
          Originally posted by Blue Baron
          so something like :
          Code:
          strnfmt(old_savefile, sizeof(old_savefile), "%s.old", path);
          while (file_exists(old_savefile) && (count++ < 100))
          {
              strnfmt(old_savefile, sizeof(old_savefile), "%s.%u%u.old", path, Rand_simple(32767), count);
          }
          except use the platform specific code where available? (file_exists has defines for platform specific code in z-file.c) err actually the Rand_simple function is defined in z-rand.h, but is not implemented. is there another random function that will not interfere with the game's rng?
          You have a good idea, but I prefer not writing independent security-related functions where possible. Alas, while most platforms have a reasonably secure function that opens a temporary file atomically, the function name and interface differ. My comments to the ticket indicate likely candidates for Unix-likes and Windows. I don't know what to do for nds.

          Comment

          • Blue Baron
            Adept
            • Apr 2011
            • 103

            #6
            Originally posted by d_m
            For some reason (under WINE) I was able to walk/run/alter using the number pad with numlock on and off after applying your fix. Are you seeing problems still?
            The plain numpad keys must be translated somewhere else. I was able to get the tunnelling working with numlock on by mapping the numpad keys to the characters used when numlock is off. But not running, probably where ever the plain keypresses are handled, trapping the WM_KEYDOWN message when shift is pressed, because I am not seeing a numpad keypress in handle_keydown when shift is pressed.

            To say it another way, when numlock is on, handle_keydown() is seeing all of the numpad key presses except when shift is held down.

            for the tunnelling I added lines like "case VK_NUMPAD7: if (mc||ma||ms) {ch = KC_HOME; kp = TRUE;} break;". When I had "ch = VK_NUMPAD7" in there various other commands were being executed, so the VK_ codes are can't be used directly by term_keypress().

            Edit: It seems like with the shift key down, and num lock is on, windows is translating the numpad keys to the numpad keys with numlock off with shift off.
            Last edited by Blue Baron; June 19, 2011, 10:19.

            Comment

            • d_m
              Angband Devteam member
              • Aug 2008
              • 1517

              #7
              Originally posted by Blue Baron
              so something like :
              Code:
              strnfmt(old_savefile, sizeof(old_savefile), "%s.old", path);
              while (file_exists(old_savefile) && (count++ < 100))
              {
                  strnfmt(old_savefile, sizeof(old_savefile), "%s.%u%u.old", path, Rand_simple(32767), count);
              }
              except use the platform specific code where available? (file_exists has defines for platform specific code in z-file.c) err actually the Rand_simple function is defined in z-rand.h, but is not implemented. is there another random function that will not interfere with the game's rng?
              I think for this it's best to use mkstemp on Unix. I found a page with Unix-to-Windows conversion ideas:



              It looks like _mktemp_s() might be the thing to us. I think this is the strategy that the real world uses nowadays.
              linux->xterm->screen->pmacs

              Comment

              • d_m
                Angband Devteam member
                • Aug 2008
                • 1517

                #8
                Originally posted by Blue Baron
                The plain numpad keys must be translated somewhere else. I was able to get the tunnelling working with numlock on by mapping the numpad keys to the characters used when numlock is off. But not running, probably where ever the plain keypresses are handled, trapping the WM_KEYDOWN message when shift is pressed, because I am not seeing a numpad keypress in handle_keydown when shift is pressed.
                Huh, this must be a place where WINE and Windows diverge I was able to hook up a keyboard to my Linux netbook and saw all the numpad stuff working. Sigh.
                linux->xterm->screen->pmacs

                Comment

                • Blue Baron
                  Adept
                  • Apr 2011
                  • 103

                  #9
                  Originally posted by d_m
                  Huh, this must be a place where WINE and Windows diverge I was able to hook up a keyboard to my Linux netbook and saw all the numpad stuff working. Sigh.
                  I guess so Shift numpad with numlock on did not run, but walked, in angband-ef5f426, angband 3.2.0, and Z+ for me. Does it work for anyone in windows in any version or variant?

                  Also, the special_key_list in 3.2.0 captures VK_SELECT, VK_PRINT, VK_EXECUTE,VK_SNAPSHOT,VK_HELP, and VK_16-VK_24, but handle_keydown() does not. Does anyone care about these keys?

                  Edit:
                  Originally posted by d_m
                  I think for this it's best to use mkstemp on Unix. I found a page with Unix-to-Windows conversion ideas:



                  It looks like _mktemp_s() might be the thing to us. I think this is the strategy that the real world uses nowadays.
                  I understand the name collision part but not the security part. Is there anything I can read about it?

                  Anyways, above i meant something like:
                  Code:
                  #if (defined(POSIX)) // I do not know what should be here file_exists() uses #ifdef HAV_STAT
                    strnfmt(old_savefile, sizeof(old_savefile),"%sXXXXXX", path);
                    res =  mkstemp(old_saevfile);
                  #elif (defined(WINDOWS))
                    strncopy(old_savefile, path,1024);
                    res =  tmpnam_s(old_savefile, 1024); // or _mktmp_s()
                    // However mkstemp opens a file descriptor, but these two don't. so there would be additional
                    //  code either here or for the unix version
                  #else
                    strnfmt(old_savefile, sizeof(old_savefile), "%s.old", path); 
                    while (file_exists(old_savefile) && (count++ < 100))
                    {
                        strnfmt(old_savefile, sizeof(old_savefile), "%s.%u%u.old", path,   Rand_simple(32767), count);
                    }
                  #endif
                  and something similar for the new_savefile name.
                  Last edited by Blue Baron; June 19, 2011, 17:28.

                  Comment

                  • myshkin
                    Angband Devteam member
                    • Apr 2007
                    • 334

                    #10
                    Originally posted by Blue Baron
                    I understand the name collision part but not the security part. Is there anything I can read about it?
                    Here is a decent summary of the potential security issues surrounding temporary file creation.

                    Comment

                    • Blue Baron
                      Adept
                      • Apr 2011
                      • 103

                      #11
                      Originally posted by myshkin
                      Here is a decent summary of the potential security issues surrounding temporary file creation.
                      Thank you, that was interesting.

                      I still don't think I really understand the security issues, but I've attached are some diff files that I think acknowledges some of the issues. It uses simple random filenames, does not delete anything unless it moved the file to the filename, uses open with the create and exclusive flags, and fails if a file already exists. It doesn't use mkstemp, because I didn't want to worry about the different platforms.

                      If it is not sufficient, you might want to add a MODE_WRITE_TEMP and add something like the following to file_open:
                      Code:
                          case MODE_WRITE_TEMP:
                          {
                              int fd = mkstemp(buf);
                              if (fd < 0) {
                                  f->fh = NULL;
                              } else {
                                  f->fh = fdopen(fd, "wb");
                              }
                          }
                      and use MODE_WRITE_TEMP instead of MODE_WRITE in the file_open call in savefile_save().

                      By the way, there are some implementations of mkstemp that could be used for the platforms that don't have it at:
                      ftp://ftp.math.utah.edu/u/ma/hohn/li...port/mkstemp.c
                      and
                      Attached Files

                      Comment

                      • Pete Mack
                        Prophet
                        • Apr 2007
                        • 6883

                        #12
                        Temp file collision is only a security issue when there's a vector for malicious use. Since angband is not executing arbitrary code, there isn't one here.

                        Comment

                        • Blue Baron
                          Adept
                          • Apr 2011
                          • 103

                          #13
                          Originally posted by Pete Mack
                          Temp file collision is only a security issue when there's a vector for malicious use. Since angband is not executing arbitrary code, there isn't one here.
                          If it doesn't get in the way of the game, it should be made more secure just on general principles/software practices. (you never know when a sys admin will play the game when they shouldn't on an account that they shouldn't ) I think the security worry is an attacker making a link between what the game uses for a temporary file and some important file, then the game overwriting or deleting that file when writing/deleting the temporary files.

                          Also, for ticket #1462, when the visuals are written while in a graphics mode, what is written of course uses that graphics mode. When the info is read back in at save file load, if the game is already using a different graphics mode, the information read will be wrong. To ignore this the way the other platforms do and avoid the crash, then you can add one line to sdl_BuildTileset() and change to:
                          Code:
                          GfxInfo *info = &GfxDesc[use_graphics];
                          if (!use_graphics) return (1);
                          This should work, but I have not tried it because I do not use the SDL port.

                          Lastly, could someone give me a sequence to reproduce ticket #758? I don't know how to generate the files. When I tried saving both options and visuals, with a second character in a save file, both were saved in the same file.

                          Comment

                          Working...
                          😀
                          😂
                          🥰
                          😘
                          🤢
                          😎
                          😞
                          😡
                          👍
                          👎