Introduction

Progress reports and musings from the developers on the current gaming tools.

Moderators: dorpond, trevor, Azhrei

User avatar
Azhrei
Site Admin
Posts: 12086
Joined: Mon Jun 12, 2006 1:20 pm
Location: Tampa, FL

Re: Introduction

Post by Azhrei »

I don't see the problem with a player moving an NPC token and having that player (and perhaps all players) see the movement metric. Basically the same code that says, "can the player see the token (at all)?" will be used to determine if the distance shows up...

Phergus
Deity
Posts: 7132
Joined: Fri May 12, 2006 8:56 pm
Location: Middle of Nowhere, NM
Contact:

Re: Introduction

Post by Phergus »

One of the issues that came up in the past was when the GM was moving a token near the edge of the fog the movement counter and GM name was showing up in the exposed area even though the token itself wasn't visible to the players.

Obviously the right logic should prevent that from happening but it is something to be careful about.

User avatar
jfrazierjr
Deity
Posts: 5176
Joined: Tue Sep 11, 2007 7:31 pm

Re: Introduction

Post by jfrazierjr »

Phergus wrote:One of the issues that came up in the past was when the GM was moving a token near the edge of the fog the movement counter and GM name was showing up in the exposed area even though the token itself wasn't visible to the players.

Yea.. something like that is what I remember, but don't have all the exact details(I knew it had to do with DM and what plaeyrs saw).... which is why I said check with Phergus/Dorpond since I knew they would remember....
I save all my Campaign Files to DropBox. Not only can I access a campaign file from pretty much any OS that will run Maptool(Win,OSX, linux), but each file is versioned, so if something goes crazy wild, I can always roll back to a previous version of the same file.

Get your Dropbox 2GB via my referral link, and as a bonus, I get an extra 250 MB of space. Even if you don't don't use my link, I still enthusiastically recommend Dropbox..

username
Dragon
Posts: 277
Joined: Sun Sep 04, 2011 7:01 am

Re: Introduction

Post by username »

username wrote:
Azhrei wrote:Adding SwingWorkers usually isn't too tough, it's just no one has sat down to do it yet. :) If you need examples, there are other places in the code that do create SwingWorkers to do off-thread processing.
I decided to synchronize the access methods. That helped. Unfortunately, that did not fix the root cause.
Here's a patch. 2/3 was synchronizing, although I probably didn't catch it all. 1/3 was a wild guess and I need to delve deeper a bit to understand why this worked. The code I commented out was there presumably for some reason. So test it with some more campaign files.
Attachments
patch.txt
(6.09 KiB) Downloaded 103 times

User avatar
Azhrei
Site Admin
Posts: 12086
Joined: Mon Jun 12, 2006 1:20 pm
Location: Tampa, FL

Re: Introduction

Post by Azhrei »

username wrote:Here's a patch. 2/3 was synchronizing, although I probably didn't catch it all. 1/3 was a wild guess and I need to delve deeper a bit to understand why this worked. The code I commented out was there presumably for some reason. So test it with some more campaign files.
Can you explain why you commented it out? That code sets the map zoom level back to what it was in the campaign file after the file is finished loading...

Regarding the rest of the patch: it looks like you didn't import the Eclipse preferences. If you check the "developers notes" thread (somewhere here!) you'll find there's a set of guidelines for code contributions. Those preferences can be imported by using the file that's included under the maptool/conf directory in the project; the file is eclipse.prefs.import-me.

Looking at the content of the patch, I'm not sure I understand the problem behind loadCampaign (your comments at line 122 in Zone.java). While it's true that loadCampaign() runs in a separate thread, the GUI (i.e. the Graphical User Interface) is blocked already because the glassPane is instantiated. That's the layer that contains the "Loading campaign..." string and the semi-transparent black overlay. Because the glassPane exists and absorbs all mouse and keyboard events, there is no way to interact with the UI. It is true, however, that the window could be resized and that could make additional tokens visible that weren't visible before, but I don't believe repaint events are recognized while the glassPane is active (I believe the first few lines of renderZone() check for the loading state and return prior to drawing anything; yes, they do: see ZoneRenderer.java:902).

If that's the case, how can the loadCampaign() cause a problem?

You said in a previous posting, "While adding some debug prints I found a concurrency violation on Zone.tokenOrderedList in here." Perhaps your print statements were in the wrong place (such as prior to the IF statement in renderZone()?).

I don't believe these concurrency patches are relevant, but I'd be happy to be proven wrong. :)

username
Dragon
Posts: 277
Joined: Sun Sep 04, 2011 7:01 am

Re: Introduction

Post by username »

Azhrei wrote:Can you explain why you commented it out? That code sets the map zoom level back to what it was in the campaign file after the file is finished loading...
It solved the problem :-) Apparently something else in the code sets it before the loading thread does. But since I don't know what or whether this alway happens, I have asked for tests. I don't have many campaign files at hand. All that is needed is load them and check that everything is alright.
Azhrei wrote:Regarding the rest of the patch: it looks like you didn't import the Eclipse preferences. If you check the "developers notes" thread (somewhere here!) you'll find there's a set of guidelines for code contributions. Those preferences can be imported by using the file that's included under the maptool/conf directory in the project; the file is eclipse.prefs.import-me.
You are right, I didn't import them. I now did, but nothing happened. (I hope you are not complaining about the unused ZoneRenderer; this is not the final patch.) On the other hand I didn't find the thread you mentioned. I did a global search and stepped through the "developer notes" forum to no avail. Maybe you can provide more detailed directions?
Azhrei wrote:Looking at the content of the patch, I'm not sure I understand the problem behind loadCampaign (your comments at line 122 in Zone.java). While it's true that loadCampaign() runs in a separate thread, the GUI (i.e. the Graphical User Interface) is blocked already because the glassPane is instantiated. That's the layer that contains the "Loading campaign..." string and the semi-transparent black overlay. Because the glassPane exists and absorbs all mouse and keyboard events, there is no way to interact with the UI. It is true, however, that the window could be resized and that could make additional tokens visible that weren't visible before, but I don't believe repaint events are recognized while the glassPane is active (I believe the first few lines of renderZone() check for the loading state and return prior to drawing anything; yes, they do: see ZoneRenderer.java:902). If that's the case, how can the loadCampaign() cause a problem?
The comment was wrong. SwingWorker is to be replaced with EDT. No, the GUI is not blocked. You are just greying everything with it. There are several issues here, which as I said have not completely understood.

(a) The map panel is being checked within the repaints (isLoading) and is independent of the actual loading thread. A separate thread is started each time, requesting some asset ids. I wonder whether the loading thread doesn't have stuff to do, after the asset ids are all available.

(b) Other panels are still visible and that's where I have seen repaint events are coming in, while loading is not completely done. There's macro processing and what not going on in the background that has effect on the tokens. That is why I needed to synchronize access to the orderedTokenList. The visible area is probably a corollary to that. I don't really know. But someone must have been suspicious, because of the trace string "ZoneView: visible == null. Please report this on our forum @ forum.rptools.net. Thank you!" (BTW, I didn't do any resizing.) Next, lot's of PropertyChangeListeners fire and I can't really follow how they cascade down into the system and obliterate the EDT.
Azhrei wrote:You said in a previous posting, "While adding some debug prints I found a concurrency violation on Zone.tokenOrderedList in here." Perhaps your print statements were in the wrong place (such as prior to the IF statement in renderZone()?). I don't believe these concurrency patches are relevant, but I'd be happy to be proven wrong. :)
No, they were not in the wrong place. They can never be. All they do is change the timing of the threads and from a programming point this is opaque. You should never rely on assumed behaviour of the scheduler. I hope the explanation above is sufficient to justify the synchronized additions.

My current problem is that I don't know who set the correct scale before the loading thread did and will "he" always do so, or is this yet another racing condition. Gotta run though, I'll check that later.

username
Dragon
Posts: 277
Joined: Sun Sep 04, 2011 7:01 am

Re: Introduction

Post by username »

tokenLocationCache in ZoneRenderer contains junk data at the first paintComponent for this campaign-file. Anybody have a clue about the intended lifecycle of that member? I still think there's a race condition at the heart of the matter. My current patch clears it at the beginning of renderTokens and that works.

I also found out that the setting of the ZoneScale, the removal of which was part of my first patch, is actually already done one or two lines before the commented line. So it can stay commented out. That makes the affair even more mysterious. There might be a validity toggling of the cache.

User avatar
Azhrei
Site Admin
Posts: 12086
Joined: Mon Jun 12, 2006 1:20 pm
Location: Tampa, FL

Re: Introduction

Post by Azhrei »

username wrote:You are right, I didn't import them. I now did, but nothing happened.
Hm. They should've set up things like code formatting defaults and whether to import individual classes or entire packages. And yes, I didn't point to the specific thread 'cuz I was lazy. I'll find it. I think it's called Developer Guidelines and it comes from the days when I was submitting patches to Trevor and I wanted to know what coding style he preferred/wanted. :)
The comment was wrong. SwingWorker is to be replaced with EDT. No, the GUI is not blocked. You are just greying everything with it. There are several issues here, which as I said have not completely understood.
Ah. Semantics. I said the "user interface" was blocked, and it is. I did not say that the EDT was blocked. To me a user interface is something the user, er, "interfaces" with. And if all events are being ignored then there's no interfacing going on. I was probably being too literal. :)
(a) The map panel is being checked within the repaints (isLoading) and is independent of the actual loading thread. A separate thread is started each time, requesting some asset ids. I wonder whether the loading thread doesn't have stuff to do, after the asset ids are all available.
I will admit that I don't fully understand the image loading process, but I thought there was a single queue and requests were added to that queue, then the queue was emptied by spawning off threads using one of the (newer) Concurrency libraries. It's certainly possible that those threads wake up code elsewhere, but IIRC they pass an ImageObserver that sets a flag that tells the Concurrency module to generate a window exposure event so that the Zone is repainted. But the Concurrency module doesn't do that until the queue is empty. The end result being that all image loading happens and the Zone isn't repainted until the threads are done. If repaint() is being called prior to that, then the synchronization problem is likely right there...
(b) Other panels are still visible and that's where I have seen repaint events are coming in, while loading is not completely done.
Yeah, that sounds like an ImageObserver triggering repaint events. Sigh.
But someone must have been suspicious, because of the trace string "ZoneView: visible == null. Please report this on our forum @ forum.rptools.net. Thank you!"
Yeah, we had bug reports on it so there were two ways to fix it and I took the simple solution of ignoring it. Bad choice. ;)

But given that I don't really grok the ImageManager and related subsystem 100%, that seemed safer. Especially given how late we are in the 1.3 build cycle.
username wrote:tokenLocationCache in ZoneRenderer contains junk data at the first paintComponent for this campaign-file. Anybody have a clue about the intended lifecycle of that member?
Not really although I seem to believe that it's generated when renderZone() is called and updated until map panning causes different tokens to be within the visible portion of the Zone component.
My current patch clears it at the beginning of renderTokens and that works.
Yeah, but wiping the cache at ever repaint is going to kill the performance of the code that checks to see if there are visible tokens nearby (mostly used by MTscript but isTokenVisible() is called from a lot of places and I believe it uses that cache; could be wrong though, as this is from memory).
I also found out that the setting of the ZoneScale, the removal of which was part of my first patch, is actually already done one or two lines before the commented line. So it can stay commented out. That makes the affair even more mysterious. There might be a validity toggling of the cache.
What do you mean by "validity toggling"?

Good work on this so far! I'm glad someone has taken the effort to look into it -- thanks!

username
Dragon
Posts: 277
Joined: Sun Sep 04, 2011 7:01 am

Re: Introduction

Post by username »

Azhrei wrote:
username wrote:You are right, I didn't import them. I now did, but nothing happened.
Hm. They should've set up things like code formatting defaults and whether to import individual classes or entire packages. And yes, I didn't point to the specific thread 'cuz I was lazy. I'll find it. I think it's called Developer Guidelines and it comes from the days when I was submitting patches to Trevor and I wanted to know what coding style he preferred/wanted. :)
Where can I see, whether anything happened at all? Because when importing preferences, it was asking for a "*.epf", which the import-me was not. (At least it didn't find it until I changed the extension to look fo "*.*".
The comment was wrong. SwingWorker is to be replaced with EDT. No, the GUI is not blocked. You are just greying everything with it. There are several issues here, which as I said have not completely understood.
Ah. Semantics. I said the "user interface" was blocked, and it is. I did not say that the EDT was blocked. To me a user interface is something the user, er, "interfaces" with. And if all events are being ignored then there's no interfacing going on. I was probably being too literal. :)
The distinction is important. The EDT does not only react to user interaction but to programmatically issued redraw events as well.
(a) The map panel is being checked within the repaints (isLoading) and is independent of the actual loading thread. A separate thread is started each time, requesting some asset ids. I wonder whether the loading thread doesn't have stuff to do, after the asset ids are all available.
I will admit that I don't fully understand the image loading process, but I thought there was a single queue and requests were added to that queue, then the queue was emptied by spawning off threads using one of the (newer) Concurrency libraries. It's certainly possible that those threads wake up code elsewhere, but IIRC they pass an ImageObserver that sets a flag that tells the Concurrency module to generate a window exposure event so that the Zone is repainted. But the Concurrency module doesn't do that until the queue is empty. The end result being that all image loading happens and the Zone isn't repainted until the threads are done. If repaint() is being called prior to that, then the synchronization problem is likely right there...
I haven't seen any image problems. Concurrency was with the members I mentioned and that came from PropertyChanges, I believe.. I also haven't seen any concurrency library being used. But then I might be foraging in old code.
But someone must have been suspicious, because of the trace string "ZoneView: visible == null. Please report this on our forum @ forum.rptools.net. Thank you!"
Yeah, we had bug reports on it so there were two ways to fix it and I took the simple solution of ignoring it. Bad choice. ;)
But given that I don't really grok the ImageManager and related subsystem 100%, that seemed safer. Especially given how late we are in the 1.3 build cycle.
Well, do you want the bug fixed or not? The synchronized is a sure thing here. I am certain that you will not see that string again, as long as the synchronizing is observed.
username wrote:tokenLocationCache in ZoneRenderer contains junk data at the first paintComponent for this campaign-file. Anybody have a clue about the intended lifecycle of that member?
Not really although I seem to believe that it's generated when renderZone() is called and updated until map panning causes different tokens to be within the visible portion of the Zone component.
It is also my impression that the cache is supposed to be used only during the rendering process. But during that rendering process no changes should be made to positions and the like. And after rendering you might as well clear the cache. Therefore all other access to the cache should be removed. That will solve the original problem I set out to tackle. But before changing that behaviour I wanted to get some feedback from the people who "had been there".
My current patch clears it at the beginning of renderTokens and that works.
Yeah, but wiping the cache at ever repaint is going to kill the performance of the code that checks to see if there are visible tokens nearby (mostly used by MTscript but isTokenVisible() is called from a lot of places and I believe it uses that cache; could be wrong though, as this is from memory).
I haven't noticed any difference. Do you have any specific performance tests? I noticed a lot of timers in there but don't know how to read them. I also don't know how my HW relates to the weakest HW you intend to release on. I have Ubuntu 10.1 on i686 (and ran maptool out of the box, ie. no memory increase).
I also found out that the setting of the ZoneScale, the removal of which was part of my first patch, is actually already done one or two lines before the commented line. So it can stay commented out. That makes the affair even more mysterious. There might be a validity toggling of the cache.
What do you mean by "validity toggling"?
I have traced the problem into the renderToken routine. Sometimes the locations for tokens (more precisely, objects on the BG layer; but they all go by the name of token in the code as far as I can tell) are available in the cache, sometimes they are not. For "general repaints" the locations are there, for rescaling events they need to be generated. But for some reason the initial "general redraw" has invalid locations. Off screen? I don't really know and I don't know what separates the rescaling repaints from the ordinary, eg. initiated through map shifts.

Whatever the case may be. It is very tedious to find out the deeper roots for this. As you said, 1.3 is in its final phase. I'd rather carve out the cache and use it locally in paintComponents only.

How does 1.4 profit from 1.3 fixes?

User avatar
Azhrei
Site Admin
Posts: 12086
Joined: Mon Jun 12, 2006 1:20 pm
Location: Tampa, FL

Re: Introduction

Post by Azhrei »

username wrote:Where can I see, whether anything happened at all? Because when importing preferences, it was asking for a "*.epf", which the import-me was not. (At least it didn't find it until I changed the extension to look fo "*.*".
It should have updated the user preferences. In particular, the ones regarding how code was formatted (indentation style and which column to insert line breaks) and how the tool modifies code (adding import statements for individual classes instead of all in a package, and trimming spaces from the ends of lines when saving).

Some of those are not going to be visible (like spaces at the ends of lines) and others are only applied when something changes (like the import statements). You can verify the import by deleting the import that uses an "*" and then saving the file and seeing how Eclipse regenerates the import statements.
The distinction is important. The EDT does not only react to user interaction but to programmatically issued redraw events as well.
Exactly -- it is important. Which is why I said "GUI" and not "EDT". You are right that repaint events can still be delivered while the "GUI" is blocked because the EDT is not blocked. I was not aware that repaint events are still generated while the GUI is blocked. I'll need to figure out why that would be happening and not being deferred until later.
I haven't seen any image problems. Concurrency was with the members I mentioned and that came from PropertyChanges, I believe.. I also haven't seen any concurrency library being used. But then I might be foraging in old code.
Image problems are the visual manifestation of (most) concurrency bugs in MT. (Obviously, since it's primary purpose is the display of graphics.)

Yeah, the concurrency library stuff is pretty isolated and was fairly recent. I don't recall where it's actually located...
Well, do you want the bug fixed or not? The synchronized is a sure thing here. I am certain that you will not see that string again, as long as the synchronizing is observed.
Of course.
[...] Therefore all other access to the cache should be removed.
Sounds good to me. :) Obviously it was added for a reason so I'll need to look at the patch to determine if there's a side effect somewhere, but this sounds like the way to go.
I haven't noticed any difference. Do you have any specific performance tests? I noticed a lot of timers in there but don't know how to read them.
When running MT go to the menus and choose Collect Performance Data. I think it's on the Tools menu, or maybe the View menu. Now you'll be able to measure the performance difference.
I also don't know how my HW relates to the weakest HW you intend to release on. I have Ubuntu 10.1 on i686 (and ran maptool out of the box, ie. no memory increase).
I'm not too concerned about that. We have some folks running MT on Java 5 on really old PowerMacs, but they're going to be dead when we move to 1.4 anyway. I'd like them to have a usable 1.3 though.
What do you mean by "validity toggling"?
I have traced the problem into the renderToken routine. Sometimes the locations for tokens (more precisely, objects on the BG layer; but they all go by the name of token in the code as far as I can tell) are available in the cache, sometimes they are not. For "general repaints" the locations are there, for rescaling events they need to be generated. But for some reason the initial "general redraw" has invalid locations. Off screen? I don't really know and I don't know what separates the rescaling repaints from the ordinary, eg. initiated through map shifts.
Hm. I vaguely recall the code you're talking about, but I'll need to look at it to be reminded. I believe the background layer is rescaled and saved in a BufferedImage so that later repaints can simply blit the image into the visible component. Changing the zoom level requires the BufferedImage to be regenerated. Things that users can interact with during normal operation are not part of the BufferedImage (since that would make them more different to manipulate). The same applies to the fog of war re: caching the results.
It is very tedious to find out the deeper roots for this.
You've got that right! Every time I go into the code I find out something else I didn't know about.
As you said, 1.3 is in its final phase. I'd rather carve out the cache and use it locally in paintComponents only.
As above... Go for it.
How does 1.4 profit from 1.3 fixes?
Good question. We expect that much of 1.4 will be written from scratch by the time a final build comes out. There are ideas for redoing the VBL, redoing the way campaigns/maps/tokens are saved and loaded, and taking advantage of new technology like JavaFX. Since it will be built on an OSGi base, much of the internal plumbing will be different.

I doubt anything done to 1.3 at this point will be carried over as anything more than a learning experience.

username
Dragon
Posts: 277
Joined: Sun Sep 04, 2011 7:01 am

Re: Introduction

Post by username »

Azhrei wrote:
username wrote:Where can I see, whether anything happened at all? Because when importing preferences, it was asking for a "*.epf", which the import-me was not. (At least it didn't find it until I changed the extension to look fo "*.*".
It should have updated the user preferences. In particular, the ones regarding how code was formatted (indentation style and which column to insert line breaks) and how the tool modifies code (adding import statements for individual classes instead of all in a package, and trimming spaces from the ends of lines when saving).
I am using Eclipse SDK 3.5.2 amd when imprting it says it can't import individual preferences. The set is "empty". Anyway, I replaced the *-imports and blanks with tabs. Hope that was it.
[...] Therefore all other access to the cache should be removed.
Sounds good to me. :) Obviously it was added for a reason so I'll need to look at the patch to determine if there's a side effect somewhere, but this sounds like the way to go.
When finding out how this cascades into other classes, I got worried and took the least invasive approach and only cleared the cache in renderToken. I have the feeling that the cache is not koscher and wrote a comment to that effect.

Attached the patch for the synchronized bits and the main one.
Attachments
patch.txt
(5.93 KiB) Downloaded 100 times

username
Dragon
Posts: 277
Joined: Sun Sep 04, 2011 7:01 am

Re: Introduction

Post by username »

Here's the other one you asked for. It contains the SystemProperties at the end of the About box/page.
Attachments
patch.txt
(2.11 KiB) Downloaded 111 times

User avatar
Azhrei
Site Admin
Posts: 12086
Joined: Mon Jun 12, 2006 1:20 pm
Location: Tampa, FL

Re: Introduction

Post by Azhrei »

Thanks. I'll take a look tomorrow night. :)

username
Dragon
Posts: 277
Joined: Sun Sep 04, 2011 7:01 am

Re: Introduction

Post by username »

Azhrei wrote:Thanks. I'll take a look tomorrow night. :)
I guess real life took its toll?

User avatar
Azhrei
Site Admin
Posts: 12086
Joined: Mon Jun 12, 2006 1:20 pm
Location: Tampa, FL

Re: Introduction

Post by Azhrei »

Yep, sorry. I was going to try to get a build out tonight but now it's not going to happen. I'm shooting for tomorrow night now. :|

Post Reply

Return to “Developer Notes”