Karazhan: netherspite portal implementation

zeroworld commented 11 years ago

for questions and feedback u can also contact baum at

http://cmangos.net/index.php http://www.scriptdev2.com/forum.php

xfurry commented 11 years ago

There are a few things which I don't like about this patch:

Lack of documentation. It's pretty hard to understand what are you trying to do in each function.
You have some things which belong to the database. Such as unit flags or factions. We change flags / factions in SD2 only when this is required. In this case it should be set in the database.
You shouldn't store pointers such as Creature* or Unit*.
There is an easy way to handle the portals using the script effect spells. I wrote a script myself about this about 1 year ago but I didn't complete it. When I'll find it I will share it with you, so maybe we can dig this up together.
You should open the pull to the master repo: https://github.com/scriptdev2/scriptdev2/commits/master Then we'll backport it to TBC.

zeroworld commented 11 years ago

Thanks for your feedback xfurry!

I would really appreciate to work with you together.

Regarding to your post: I did the database stuff in the script because otherwise you cannot test it. Is there a way to push database changes here? Furthermore please tell me how to deal with pointers in sd2 correctly.

Finally i will try to documentate the script in detail and then i will pull it to the master repo ;)

best regards baum

Schmoozerd commented 11 years ago

database changes cannot be pushed here, but they are not expected to be handled with sd2. Usually we add them as comments into github - in your case it might be ok this way (as you say: need for test) Though database changes we suggest to the DB providers as script-requirements must be properly confirmed as correct. Elsewise we will get problems again when the custom (wrong) data will be updated from the DB teams again with correct data. Belief me/us - being dependend on DB data (that is not clearly obtained from official) is horrrrriable.
Poiinters: We store ObjectGuid variables with which you can get pointers to npcs/players or gameobjects. For unique npcs/Gos easiest with the instance script (compare door handling)
Documenting is nice. Also keep in mind that commit-messages within pull-requests can be a good place to explain what you are doing why with what reason
In long term when you are really interested in developing scripts and helping us improving SD2, working on the main repo (wotlk) is the best way; however it is not required - but it will take more time if we need to confirm things over different client versions and adapt things. In general we from SD2 are very happy that our code is pretty much identical over the different clients :)

zeroworld commented 11 years ago

Thank you for your feedback Schmoozerd!

I going to set up wotlk the next days. Portal implementation update is also on the way.

best regards baum

xfurry commented 11 years ago

I found the two old patches I wrote for Netherspite portals. None of them is complete but I think that a combination of these 2 can do the job. Here is my work: http://paste2.org/UcMM9JOb http://paste2.org/ns6zKmPf

xfurry commented 8 years ago

Implemented in core. Will be backported to TBC soon.

scriptdev2 / scriptdev2-tbc

Karazhan: netherspite portal implementation #6