Menu
▾
▴
[Pcgen-trackers] [ pcgen-Bugs-1165456 ] [5.8RC2]SOURCExxx tags ignored on .MOD line
|
From: SourceForge.net <no...@so...> - 2005-04-12 09:27:06
|
Bugs item #1165456, was opened at 2005-03-17 18:59 Message generated for change (Comment added) made by zaister You can respond by visiting: https://sourceforge.net/tracker/?func=detail&atid=384719&aid=1165456&group_id=25576 Category: Other Group: None Status: Closed Resolution: Fixed Priority: 9 Submitted By: Ravagon (ravagon) Assigned to: Stefan Radermacher (zaister) Summary: [5.8RC2]SOURCExxx tags ignored on .MOD line Initial Comment: From #pcgen @ irc.dal.net [CM]Zaister each MOD line must be able to remember the last global SOURCE tag that were "active" at the moment the MOD was read ---------------------------------------------------------------------- >Comment By: Stefan Radermacher (zaister) Date: 2005-04-12 11:27 Message: Logged In: YES user_id=435051 In the current implementation, that is in RC6, both SOURCELONG:xxx|SOURCESHORT:yyy|SOURCEPAGE:p.z and SOURCELONG:xxx <tab> SOURCESHORT:yyy <tab> SOURCEPAGE:p.z are acceptable. However, it seems, Éric wants to have SOURCELONG, SOURCESHORT and SOURCEWEB disabled on object lines. Mind you, the CMP tags have not been able to change anything uintil now. ---------------------------------------------------------------------- Comment By: Paul W. King (kingpaul) Date: 2005-04-12 11:07 Message: Logged In: YES user_id=277877 I know that the CMP datafiles have SOURCELONG:xxx|SOURCESHORT:yyy|SOURCEPAGE:p.z on all their object lines. From your statement, should it rather be: SOURCELONG:xxx <tab> SOURCESHORT:yyy <tab> SOURCEPAGE:p.z Paul W. King TM SB, OGL/PL Chimp, Data Gibbon, BoD ---------------------------------------------------------------------- Comment By: Stefan Radermacher (zaister) Date: 2005-04-12 10:48 Message: Logged In: YES user_id=435051 Oh, I was not aware that these were not supposed to work there - they were ignored the same as SOURCEPAGE was, so I made it so that they all work on an object line now. I can easily reverse that, though, so that only SOURCEPAGE works. ---------------------------------------------------------------------- Comment By: Eric Beaudoin (ericbeaudoin) Date: 2005-04-12 01:45 Message: Logged In: YES user_id=324612 Stephan, I just want verify something here. SOURCELONG, SOURCESHORT and SOURCEWEB are not supposed to be valid on an object line. Reading what you wrote here, I don't know if you changed that or not. ---------------------------------------------------------------------- Comment By: Stefan Radermacher (zaister) Date: 2005-04-08 17:33 Message: Logged In: YES user_id=435051 This is now implemented as follows: If a .MOD line contains a SOURCExxx tag, the source for that particular object is completely replaced by the source that was specified as global in the file containing the MOD line and the SOURCExxx tags specified on the line (where the tags on the line have precedence over the global ones). If a .MOD line does not contains a SOURCExxx tag, the object's source is not modified. ---------------------------------------------------------------------- Comment By: Devon Jones (soulcatcher) Date: 2005-04-04 06:21 Message: Logged In: YES user_id=107647 ok, james says he needs some of this for another bug. Z, can you talk to james, and do what is minimally needed please? That much is approved for rc. Devon ---------------------------------------------------------------------- Comment By: Eric Beaudoin (ericbeaudoin) Date: 2005-03-30 21:36 Message: Logged In: YES user_id=324612 Well, if the only thing we can get for 5.8.0 is replacement, we'll take it for sure. I'm certainly not about to argue with you on how the code is or was done. The one thing I can tell you is that I'm 100% sure that is used to be an additive tag in the past. It's not very important how it was at this time except to remind ourselve that often, ppl change code to either fix or optimized and assume things. In many instances, stuff end up being lost in the shuffle. I'm mentionning this because the first big initiative for 5.9.1 is the move to CDOM. I guess we have to come up with a strategy to make sure we will not lose functionnality when we do that. ---------------------------------------------------------------------- Comment By: Devon Jones (soulcatcher) Date: 2005-03-30 17:37 Message: Logged In: YES user_id=107647 Eric, the issue here is that some things in pcgen are variables, some are vectors. *adding* means that instead of being a single string, it's a list. this is a core change in how it works. Potentially it may have worked before *as* a single string, but that means it was doing so as a pure hack. This would not surprise me, a pcgen in teh code in more then one place has overloaded strings to try to turn them effectivly into lists. This is bad bad form, so if it's the case that this is a single string, and we don't maintain a list of sources the object has been from, we should not be making that change in functionality now. I fully suppoirt in 5.9.1 moving to a situation where we keep source information in a list so that we know all sources that have touched a particular item. Devon ---------------------------------------------------------------------- Comment By: Eric Beaudoin (ericbeaudoin) Date: 2005-03-29 03:11 Message: Logged In: YES user_id=324612 I really meant, the default should always be to add. That's true for any .MOD that we add. I know very well that many tags do not follow that but that's another story. As for SOURCEPAGE, when it was working, it use to work the way descripbed. The example I gave you is a few lines cut from a file I have that I use to work with. I can't tell what happen at the code level or if it was done as a list or as a simple concatanation of the text but it was. I had to ask Greg to activate SOUCEPAGE:.CLEAR because when I was moding the spells, the SOURCEPAGE would be added one after the other. ---------------------------------------------------------------------- Comment By: Stefan Radermacher (zaister) Date: 2005-03-29 03:01 Message: Logged In: YES user_id=435051 Éric, Tir, in fact making SOURCEPAGE a list would be changing funtionality. That has not been in the code. Basically, Bryan disabled modding the source, but even before that, there was only one entry possible. I have only reenabled the change that Bryan made (and added a fix for the problem why he disabled it in the first place), but it was not possible to have more than one source for any one object previous to Bryan's change. As far as "the default always being to add", that's not really correct, there are lots of tags that only allow one value, which gets replaced by a MOD. However - I agree, a list makes more sense with SOURCExxx tags - so it could read e.g. "PHB, p. 123; CW, p. 45" - BUT that will be new functionality and will be changing the way it worked before. (Also, I recommend using ";" as delimiter, since the comma is already in use between name and page.) Also, you may have understood me wrong. When I said "as currently it will change the source but keep the original page number, i.e. it would say "Complete Monkey, SpellsM.rtf"" I was not implying that this should be the new way it works, only that my implementation currently works that way. I just wanted to make clear that it is not yet finished to commit. ---------------------------------------------------------------------- Comment By: Andrew McDougall (tir-gwaith) Date: 2005-03-29 02:46 Message: Logged In: YES user_id=208239 I agree with Eric. standard for changing a SOURCEPAGE should be a SOURCEPAGE:.CLEAR SOURCEPAGE:New Sourcepage Allows for some nice interesting "strange monkey" abilities to comment on if an object has serious portions from more than one source. ---------------------------------------------------------------------- Comment By: Eric Beaudoin (ericbeaudoin) Date: 2005-03-29 02:32 Message: Logged In: YES user_id=324612 Please Zaister, do not change the functionnality that used to be there. I know it is silly for tag like SOURCEPAGE but the default should always be that tag add to one another, they do not replace. We have the .CLEAR syntax for clearing but we do not have any proper syntax for merging. Also, there case when we needed to add with SOURCEPAGE, souch as an entry that is define in one book and modified in another with an optionnal rule. The merge for SOURCEPAGE should add the "," between the entries. ---------------------------------------------------------------------- Comment By: Stefan Radermacher (zaister) Date: 2005-03-29 02:25 Message: Logged In: YES user_id=435051 Éric, the files you attached work with what I currently have. Howver consider adding a file that says this: ---- snip ---- SOURCELONG:Complete Monkey|SOURCESHORT:CM Mage Armor.MOD CLASSES:CodeMonkey=1 ---- snip ---- as currently it will change the source but keep the original page number, i.e. it would say "Complete Monkey, SpellsM.rtf" I'll work on it. btw, you won't need the SOURCEPAGE:.CLEAR ---------------------------------------------------------------------- Comment By: Eric Beaudoin (ericbeaudoin) Date: 2005-03-28 21:59 Message: Logged In: YES user_id=324612 When you are done, be sure to test with the files I have attached. The result should be that the source information will be changed by the SOURCEPAGE on the .MOD line and the second SOURCELONG/SOURCESHORT at the top of the file. ---------------------------------------------------------------------- Comment By: Stefan Radermacher (zaister) Date: 2005-03-28 21:32 Message: Logged In: YES user_id=435051 OK I'll work on my code so it works as you guys propose :) ---------------------------------------------------------------------- Comment By: Eric Beaudoin (ericbeaudoin) Date: 2005-03-28 19:04 Message: Logged In: YES user_id=324612 I have the same expectations as Tir here since this is how is used to work. No SOURCEPAGE tag, no change. SOURCEPAGE tag present, change it with the current SOURCELONG/SOURCESHORT + SOURCEPAGE. We've used (and still) extensive .MOD files for SPELL in order to add class and domain spells. In these case, we do not want to change the source information, we just add class that can cast it. I'm pretty sure there are still some of these haging around even though there is a SPELLLEVEL tag now. ---------------------------------------------------------------------- Comment By: Andrew McDougall (tir-gwaith) Date: 2005-03-28 18:11 Message: Logged In: YES user_id=208239 Why would you put a SOURCEPAGE tag on an item when you were changing it? To me, if you are adding a SOURCEPAGE tag, you mean for the reference to be the new source. If the tag is left off, you are adding something (like a new link, or changing a tag to take into account abilities that affect that tag's function/display for a rule in the new source) but not inherently changing the item. To me, if there is a SOURCEPAGE:p. tag in the line, the complete SOURCE info for the object should be changed. If an item is .MOD'd but no SOURCExxx tag, then the SOURCExxx tags should be left alone for the object. ---------------------------------------------------------------------- Comment By: Stefan Radermacher (zaister) Date: 2005-03-28 17:45 Message: Logged In: YES user_id=435051 Eric, I must warn though, that this will affect certain source files in maybe unexpected ways. Some sources use this way to add an existing spell to a new class: SOURCELONG:Some Supplement|SOURCESHORT:XYZ Some Spell.MOD CLASSES:New Class=1 SOURCEPAGE:p.123 With my code changes, this will then change the SOURCELONG and other tags to the global tags active tags, i.e. the spell will then be displayed as coming from "Some Supplement, p.123" which is undesirable. According to Tir, adding an existing spell to a class should be done with SPELLLEVEL in the class file, so the problem of the "worng" souce being displayed would not occur. HOWEVER, it seems SPELLLEVEL can only expand on spells that have CLASSES: or DOMAINS: set on their spell line. If a spell does not have a CLASSES: tag, it cannot be added to another class by the SPELLLEVEL tag. This is a bug I have not yet fixed, and wicht would be necessary to switch to the moddable Source tag system. ---------------------------------------------------------------------- Comment By: Eric Beaudoin (ericbeaudoin) Date: 2005-03-28 16:55 Message: Logged In: YES user_id=324612 If the code is ready, I will try to appeal to Devon. Devon, although you say it's not critical, it will be a major thing for ppl that will go from older versions to 5.8. This is a functionnality that was there before and that was remove. Most ppl don't print the reference documents, they bring their WotC books to the table. Our page references are usefull (a bit) for ppl who bring laptop to the table but I don't beleive we are talking the majority of gamers here. My guess is that it was broken when the PUBLISHER* tags where added to the .PCC files. Before that, we had full control over the display of the source information with the SOURCE* tags. After that, there is some hacking happening and now a publisher name is added in front. (This is another anoyance since we can't change or remove the publisher at the file level). I'm including a simple file with what used to work before. ---------------------------------------------------------------------- Comment By: Stefan Radermacher (zaister) Date: 2005-03-28 11:52 Message: Logged In: YES user_id=435051 I have the code work done for this, but Devon said to hold this up, so I'm waiting until after the 5.8 release to commit it. Simply changing the page alone would not really be enough, if you want to mod the spells to the PHB pages for example. You would then get "RSRD, p.xy" entries. ---------------------------------------------------------------------- Comment By: James Dempsey (jdempsey) Date: 2005-03-28 02:30 Message: Logged In: YES user_id=558288 I've been looking at this because it is also the cause of editors ignoring changes to source info. The change was made in v5.5.2 I believe. Anyway, SOURCEPAGE should be safe to override as it is unique to the individual item whereas there are issues with having discarded source info mean that the other source tags are riskier to deal with. Maybe just the change to SOURCEPAGE should be considered for 5.8? This would be a one line change to PObject.setSourceMap from if (sourceMap.get(key) == null) to if (sourceMap.get(key) == null || key.equals("SOURCEPAGE")) James. ---------------------------------------------------------------------- Comment By: Eric Beaudoin (ericbeaudoin) Date: 2005-03-18 03:48 Message: Logged In: YES user_id=324612 It is quite irritating and it was working find in 5.6 so the problem is related to something we did in the 5.7.x path. If it is easy to fix, I would appreciated it if it gets dealt with for 5.8. The SOURCEPAGE:xxx.rtf is the most annoying things when working with PCGEN files. I'm sure I'm not the only one with a very large collection of .MOD that only add the correct SOURCEPAGE. ---------------------------------------------------------------------- Comment By: Stefan Radermacher (zaister) Date: 2005-03-18 01:15 Message: Logged In: YES user_id=435051 OK; I'll hold hold this up until after the 5.8 release. ---------------------------------------------------------------------- Comment By: Devon Jones (soulcatcher) Date: 2005-03-17 22:35 Message: Logged In: YES user_id=107647 Denied for RC. Although this is potentially irritating, it's not critical to being able to make a character. Lets hit this in 5.9 ---------------------------------------------------------------------- Comment By: Stefan Radermacher (zaister) Date: 2005-03-17 19:09 Message: Logged In: YES user_id=435051 Devon: please decide if this shall be included in 5.8 Currently .MOD is completely ignoring any attempt to change the SOURCExxx tags. PCGen should be able to (1) accept SOURCExxx tags on the .MOD line and modify the modded object, and (2) it should also be able to accept the "current" global SOURCExxx tags that are active in the file when the .MOD line is read. Point (2) can't work currently, because MODs are only applied after all files of a type have been read, and by then the global tags are long gone and forgotten. If MOD were currently simply allowed to change SOURCExx tags, this would result in all modded objects gaining the last read global SOURCExxx tags. That is why Bryan chose to disable point (1). I have (1) fixed locally, and I am currently thinking of a solution for (2). ---------------------------------------------------------------------- You can respond by visiting: https://sourceforge.net/tracker/?func=detail&atid=384719&aid=1165456&group_id=25576 |