Easy Quest

[Moderator]

12th Dec 2015
IcemanBo: Too long as NeedsFix. Rejected.

9th Nov 2011
Bribe: Debug messages should only show in DEBUG_MODE.

Function interfaces are really bad practice for a simple design like this. Do you know that they make copies of the functions being processed? I would prefer "GetEventQuest" "GetEventQuest...Player/NewUnit/OldUnit" which runs off of the Advent system than this setup you have here.

Alternatively, a module to implement the code (like UnitIndexer uses) keeps the user verbosity light and keeps the compiled code light as well.

Polymorphism doesn't work well in structs as those "onDestroy" methods get copies as well. Better to use array structs and make custom "destroy" to avoid such a compiled mess.

Your system combines many different concepts and principles and considers it "one resource". If a user just wants a few aspects of a quest system this would be a horrible choice. Even if a user wanted a more complex system maybe they want it done completely differently from what you have. I have a very strong opinion that your system tries to be more than it is required to be.

I recommend null all handles even players. Some have reported bugs with not nulling players causing leaks in some cases.

Also don't initialize "local player p = Player(0)" when you only reference it once before the loop.

        set this.title = "Generic Quest Title"
        set this.description = "Generic Quest Description"

^ These should be at the top in a configurables-section.

The mini-library you named "Array" should just be implemented into your quest system and made a private struct, because it is nowhere near modular enough to take such a dynamic title as "Array", for one there being no "pop" method is a real stinker.

Have a look at RegisterPlayerUnitEvent, it is a really good resource for saving handle count.

 

[Magtheridon96]

Can you please post the triggers?

You'd post them in [trigger][/trigger] tags

edit
Oh .. it's vJass ^^
Excellent
Post the code in [code=jass][/code] tags

edit
- Dont use onDestroy. It's Old School
- Vexorian's Table is Old School
- Aligning without indenting after "library" is old school Jass
- Super long variable names are old school too
- Way too many public keywords :l (which is old school )
- Needs to be slightly more modular (Everything except "Quest System" shouldn't be included unless you want to give users examples of what kind of quest types they can create)

This system has alot of potential

 

[Sadalbari]

Post the code in

tags

There's a map... Why should I post jass that spans over more than 1000 lines of code and 12 triggers?

edit
- Dont use onDestroy. It's Old School
- Vexorian's Table is Old School
- Aligning without indenting after "library" is old school Jass
- Super long variable names are old school too
- Way too many public keywords :l (which is old school )
- Needs to be slightly more modular (Everything except "Quest System" shouldn't be included unless you want to give users examples of what kind of quest types they can create)

- I'm old school.
- What exactly do you mean with "more modular"? How does removing modules make a system more modular? I also think that the other triggers should be in the demo map. There's no need to keep reinventing the wheel - people WILL need a QuestItemKillUnit quest item.
Also, I need to include them to
a. demonstrate the system
b. give examples for users to create their own plugins

Lol take look on my system... Btw where it is easy? It is far not easy... Also if spell is not completed then post it to project developement Btw I am interested in that where will your system lead to so cmon!

The system is ready for use. I'm planning to add a few features and some more modules, but it's not necessary so I consider this to be completed

The easy part is in creating new quests.

    local QuestType qt = QuestType.create(gg_unit_Hpb1_0001, 'qgno')
    set qt.title = "Gnoll invasion"
    set qt.description = "The farmlands have been plagued with a gnoll invasion. The major has asked you to kill as many gnolls as possible!\n\nRepeatable."
    set qt.iconPath = "ReplaceableTextures\\CommandButtons\\BTNGnollArcher.blp"
    set qt.required = false
    set qt.unique = false
    set qt.locationX = 1380
    set qt.locationY = -190
    call qt.addCondition(QuestConditionLevel.create(2, 5))
    call qt.addCondition(QuestConditionQuest.create('qgnl'))
    call qt.addItem(QuestItemKillUnit.create('ngnb', 3, "%s/3 Gnoll Brutes killed."))
    call qt.addItem(QuestItemKillUnit.create('ngns', 3, "%s/3 Gnoll Assassins killed."))
    call qt.addItem(QuestItemKillUnit.create('ngnw', 3, "%s/3 Gnoll Wardens killed."))
    call qt.addReward(QuestRewardResource.create(150, 0))
    call qt.addReward(QuestRewardItem.create('phea', 0))

It's 15 lines of code that create a new quest, including minimap pinging, text messages, quest interface, and keeping track of conditions & questitems. You don't have to create a trigger to update a quest when a unit is killed, or another trigger to unlock a quest when a hero reaches level 2.
This specific example:
- Conditions: hero must be level 2, and have finished the "Gnoll Scout" quest (id: 'qgnl')
- requirements: hero must kill 3 gnoll brutes, 3 gnoll assassins and 3 gnoll wardens.
- rewards: hero will gain 150 gold and a healing potion

 

[Magtheridon96]

There's a map... Why should I post jass that spans over more than 1000 lines of code and 12 triggers?

Because it's a requirement?
You should only post QuestSystem and QuestHero (optional)

What exactly do you mean with "more modular"? How does removing modules make a system more modular? I also think that the other triggers should be in the demo map. There's no need to keep reinventing the wheel - people WILL need a QuestItemKillUnit quest item.

What does removing modules have to do with any of this
You should put all the triggers except QuestSystem and QuestHero in a seperate category called "Examples" or "Plugins"

Making it more modular is giving us the ability to find multiple uses for it.

For example, Nestharus' Encoder was written to be used in Save/Load systems.
But, since he didn't include anything in the actual system that limits its uses to "Save/Load", it can now be used for practically anything that has to do with encryption/compression

I know your system is totally different, but for the sake of modularity/portability, all you have to do is move all those triggers to a different category to make it "look" modular ;D


edit
Oh and you should be using Bribe's Table for this system because Vexorian's Table is ... ... .. .. bad

 

[Sadalbari]

Because it's a requirement?

Didn't see that. Will do asap.

You should only post QuestSystem and QuestHero (optional)
What does removing modules have to do with any of this

Well, *you* said I had to remove modules... But apparently you meant to say this:

You should put all the triggers except QuestSystem and QuestHero in a seperate category called "Examples" or "Plugins"

which makes more sense to me. So I guess I'll do it, although I don't think it makes any difference at all.

Making it more modular is giving us the ability to find multiple uses for it.I know your system is totally different, but for the sake of modularity/portability, all you have to do is move all those triggers to a different category to make it "look" modular ;D

Dude.
1. What does portability have to do with this?
2. What does modularity have to do with this?
Moving a few triggers to a different folder doesn't make a system modular. The fact there *are* multiple triggers does. Actually, scrap that. Having multiple triggers makes a system extensible, not modular.

Oh and you should be using Bribe's Table for this system because Vexorian's Table is ... ... .. .. bad

As far as I can see, it worked perfectly fine for the past years. In fact, to me it looks like a lot of code bloat to do something that already exists.

 

[Magtheridon96]

Moving a few triggers to a different folder doesn't make a system modular

The thing is, your system would be more modular if you treat everything except the engine as nothing but examples
Moving them to another category is the first step xD
It would also be more modular if you split up the engine into 2 parts:
- Core
- Plugin Structs (A library that requires the core)

The Plugin Structs would be where all those extra structs (The ones that have methods that need to be overloaded) are

When we say modularity in Jass, we're not expecting much
We only expect organized code with multiple uses (Not expected in this) and we also expect "ease of implementation"

Don't worry, your system's "Approval-Worthy", it just needs a bit more organization and some possible tweaks (I didnt read the code in detail so I dont know if optimizations are possible, but they usually are )

 

[Sadalbari]

Because it's a requirement?

I rechecked the rules and don't see where it says it's a requirement.

The map has been updated. Added a few more quests as examples.
New features:

• exclamation mark / question mark when units have quests available/pending/finished
• Deadline & Countdown items (quest must be finished within time)
• Various code changes not worth mentioning

 

[Magtheridon96]

I rechecked the rules and don't see where it says it's a requirement.

The map has been updated. Added a few more quests as examples.
New features:
exclamation mark / question mark when units have quests available/pending/finished
Deadline & Countdown items (quest must be finished within time)
Various code changes not worth mentioning

It might not be in the rules, but any user/moderator would require you to post the code
because there are TONS of people who can't download the map.
Posting the code makes it easier for mods to moderate your resources
98% of people who've been active in this section for more than 3-4 months never like to
download the maps :/
Infact, I just finished deleting every last spell/system map from my disk xD

 

[Adiktuz]

Mostly, coders will check first the code, then decide if the resource is worth trying...

and it also makes it easier for mods to moderate especially when they are on a computer that does not have wc3...

and mostly for vJASS resources, the only thing that you need to copy is the code (with the exception of those resources like Anachron's FSI which includes necessary imports), so having it posted here will make it faster for users to implement your resource as its a lot faster to Copy-paste than to download and open the test map...

 

[Sadalbari]

If coders are too lazy to download a map, especially with pretty much only 1 competing system on this website, they're too lazy to finish their own map anyway. It's their problem, not mine.

If moderators are going to approve a system by glancing at the code instead of checking if the testmap even compiles, let alone works, they're retarded. I don't want my resource approved by retards.

Copying the code as text would be insane because you most likely need the plugins too (and since you're all so damn lazy, it's easier to open the map and copy 3 folders than copy 17 triggers, that's excluding documentation & examples), and most definitely want to copy the unit in the object editor as well.
P.s. it *was* only 2 folders but someone made the *excellent* suggestion to split it further into 3 folders.

 

[Adiktuz]

then I could say that ur lazy too, because if not then you would have browsed the previous resources and see that they check the code first before testing the map... its like a preliminary check to see which resources are worth checking in-game...

and about the CnP thing, its generally for vjass resources, not specific for yours only... in this case, yours was an exception to that...

anyway, I agree with Magtheridon96, the system is Approvable... ^_^

 

[Geries]

Dude you are using modules and textmacros right? Then easy implement them yourself into the major script so you can post them together the rest of your code like documentation and sample quests are inneed to be posted... I only tell the same that mag told... You should post your codes. Also like others i only have a phone for two weeks so i cant open the maps. Now or later you have to post them if you want to be approved by those "retards"

 

[Magtheridon96]

I'm going to use this system in the future (After I change a few things ;p)

@Bribe, Maker & TRD:
- As soon as any optimizations worth doing are done, I recommend that you approve this system and give it a 3/5 - 4/5

 

[Bribe]

As far as I can see, it worked perfectly fine for the past years. In fact, to me it looks like a lot of code bloat to do something that already exists.

Vexorian's optimizer kills all of the textmacros, because they inline, but yeah it is quite intimidating I admit. The total code length after the optimizer (end result) will actually be smaller than Vexorian's Table.

 

[Sadalbari]

The main reason that I'm not updating the map to use Bribe's Table right now is that the flush method is incompatible with Vexorian's Table. I can work around StringTable & HandleTable, but not the flush method. (that is, not in an elegant way)
Since most existing libraries use Vexorian's table, and importing 2 Tables in your map doesn't seem like a good idea, I'm sticking with Vexorian's. If the flush method is fixed I'll fix the rest of the system so you can plug in your preferred Table library.

 

[Bribe]

Who uses his .flush() method as it was? Most of the time you'd be using .reset().

I don't even see you using the .flush() method in your triggers...

 

[Sadalbari]

The problems are:

vex.reset = bribe.flush
vex.flush = bribe.remove
vex.exists = bribe.has

If you want a new Table, I think you should at least use the same API. Especially with all other existing libraries using vex's table.
But that's not even relevant to this system. So keeping it "pending" isn't helping me...

 

[Magtheridon96]

Everyone's using Bribe's Table nowadays.
No one would use this system if it's using Vexorian's Table.

Yo Vexorian, I'm really happy for you, and Ima let you finish, but Bribe's Table is the best Hashtable API for Warcraft III

 

[Tukki]

Pro tip for free: Use Bribe's Table with the BC script.

This is probably one of the best tracking quest systems on THW.
I've done a few tweaks to make it fit a map I'm working on and so far it's working as it should.

Some notes if you're still keen on updating.

1: You should either add a new status event (onComplete?) or make onFinish fire when the quest is completed, and not when all objectives are done.
This only a real issue with follow-up quests from another quest giver, as the quest giver receives the new quest when you've completed all objectives - not returned and collected your rewards.

2: I added 'restrictions' to quest items, which is something I'd like to see in a new version.
I.e QuestItemKillUnit can use Area, Time and 'Presence' restrictions.
(Presence requires that a unit-type must be nearby, Area requires that the unit is killed within a specified area and Time requires that the unit is killed in a time window).

3: Also, I like having an option to make objective progression shared, so I edited some stuff, and it works.
Would be nice to see this being supported by default, though!

Haven't found any issues so far, and I'm really fond of the extensible design!

Great job!